[llvm-commits] [llvm] r89403 - /llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp

[Chris Lattner]

On Nov 22, 2009, at 10:28 AM, Jim Grosbach wrote:
>> Are you sure that this is the right thing to do?  GCC inline asm precisely specifies how the size of an inline asm is computed:
>> http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Extended-Asm (6.39.1)
>> 
>> If some inline asm isn't working with this algorithm, then the inline asm is wrong and should be fixed.
> 
> 
> Yes, I believe this is the correct approach.
> 
> The algorithm described is, "The estimate is formed by counting the number of statements in the pattern of the asm and multiplying that by the length of the longest instruction on that processor." This is exactly what we do, and gets us close, but it is still an estimate. To do better, we'll need to actually parse the assembly code to determine whether it's a 16 or a 32 bit instruction. In Thumb2, that's a non-trivial thing, and I suspect we don't want to go down that path, at least right now.

No, this isn't true at all.  This is a statement of how the language extension (in this case, gnu assembly) works.  If the "estimate" is incorrect for a piece of code, then the asm is incorrect, not the compiler.

> What's happening here is that the inline asm is actually shorter than the estimate since it's a 16-bit instruction, and is also not located between the constant pool reference and the constant pool entry. Since it's different in size than what the estimate believes, our calculations for when alignment padding will be inserted are now off, leading to problems.

Ah, this is a completely different problem from what I thought.  I thought we were incorrectly under-estimating the size of an asm, not overestimating it.  If the code is expecting to get the exact size of the asm, it is definitely "doomed to failure" :).  However, if you just need alignment, can you just not insert "alignment padding" and instead insert a .align directive after the asm?

> This patch adjusts those padding calculations to know that in the presence of inline assembly, we're dealing with an estimate and therefore can't make assumptions about alignment padding.


Ah ok, so this just causes extra .align directives to be emitted, it doesn't make wildly conservative assumptions?  If so, sounds great! :)  Thanks Jim,

-Chris