neo430 icon indicating copy to clipboard operation
neo430 copied to clipboard

Proable bug in neo430_bswap

Open nrother opened this issue 4 years ago • 1 comments

The current implementation of neo430_bswap seems to be buggy. In my case, the compiler selected a totally unrelated register to call swpb on (not the one containing a), which obviously caused the rest of the program to fail.

I'm also confused why swpb is called with two operands, since the instruction seems to have only one...

This code seems to work:

uint16_t neo430_bswap(uint16_t a) {

  register uint16_t r = a;
  asm ("swpb %0" : "+r" (r));
  return r;
}

Note, that I also dropped the volatile qualifier because the ASM code is "pure" (i.e. the outputs depend only on inputs). I would also recommend to move the function to the header and mark it as inline.

nrother avatar Mar 04 '21 17:03 nrother

Sorry for the late answer.

The current implementation of neo430_bswap seems to be buggy. In my case, the compiler selected a totally unrelated register to call swpb on (not the one containing a), which obviously caused the rest of the program to fail.

Lately, I have been playing a lot with inline assembly / intrinsics and maybe asm ("some instruction with some registers"): seems to be bad practice in some cases as it leaves too much freedom to the compiler to "optimize" in the wrong direction ("constant propagation" seems to be a certain issue). I am experimenting with a more sophisticated approach (-> example). I think this might be a cleaner (still hacky, somehow) and clearer way - or maybe not. I am still trying to figure it out.

I'm also confused why swpb is called with two operands, since the instruction seems to have only one...

Good point. I cannot remember why I have used two operands...

This code seems to work: ... Note, that I also dropped the volatile qualifier because the ASM code is "pure" (i.e. the outputs depend only on inputs). I would also recommend to move the function to the header and mark it as inline.

Thanks for the modification. I will check it out.

stnolting avatar Mar 25 '21 16:03 stnolting