Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

Proable bug in neo430_bswap #11

Open
nrother opened this issue Mar 4, 2021 · 1 comment
Open

Proable bug in neo430_bswap #11

nrother opened this issue Mar 4, 2021 · 1 comment

Comments

@nrother
Copy link

nrother commented Mar 4, 2021

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.

@stnolting
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants