[PATCH] switch ARM modes in inline asm
grosbach at apple.com
Sun Dec 1 23:32:00 PST 2013
On Nov 25, 2013, at 12:43 PM, Greg Fitzgerald <garious at gmail.com> wrote:
> I'm starting to think that fixing this bug is a bad idea and that reporting an error might be more appropriate. Fixing the bug opens up a can of worms. Say the user's inline assembly doesn't return to thumb mode. Should the compiler inject the $t mapping symbol? Should it inject a ".code 16" directive? Should it inject a label and adr to switch back to thumb mode? Where do we draw the line?
Note that $a/$t are ELF specific.
Personally, I don’t think this is the kind of thing that’s appropriate to do in inline assembler at all, partly for the reasons you outline above. On the other hand, people do things with inline assembly I don’t consider appropriate all the time, so opinions obviously vary. ;)
You’ve correctly identified the core assumption the streamer makes and where that breaks down when inline assembly starts playing games, and your original patch is a totally reasonable way to move towards fixing that (and is probably generally correct regardless of what we want to do for this sort of inline assembly).
I’m surprised you need to do anything at all to get the $a/$t stuff hooked up. That’s all at a lower layer and I’d have expected it to “just work.” Perhaps the ARM ELF streamer is playing additional games that break that?
To teach the compiler to recognize a missing mode switch back to ARM/Thumb from inline assembly and to then switch back via a label and branch would be extremely difficult to do properly. I would strongly prefer we just generate a diagnostic. Even that will be very tricky, as to do it right and minimize false positives would require figuring out any control flow in the inline assembly (there will be at least some to switch modes in the first place) and see if there’s any exit paths that don’t switch back. To put it mildly, nothing in our inline assembly handling is set up to do anything like that right now, so you’d be building significant infrastructure.
Pragmatically, that leaves us with two realistic options. 1) Fix the assembler mode directives to work in inline assembly and if the user abuses them and doesn’t switch back or something, they get a really nasty bug. 2) Prohibit them in inline assembly and issue a diagnostic if we see them.
I’m fine with either, but prefer (2) in a vacuum. If there’s significant extant use cases where people are doing this, then that may not be tractable.
> Also, my test is fragile. That $a shouldn't be displayed by llvm-objdump (and isn't by GNU's objdump). Also, the CHECK-NEXT assertions are checking those bytes in the context of thumb instructions. Instead there should just be one CHECK that matches all 4 bytes on one line.
More information about the llvm-commits