[PATCH] D31116: arm: don't promote too small constants

[Tim Neumann via llvm-commits]

Hi James,

a reduced testcase is attached to the linked bug report, I'm not really familiar with LLVM so wasn't sure how / where this should be included in the patch. To be honest I didn't try to understand the complete promoteToConstantPool, I just wanted to try and fix the bug since it is blocking Rust's LLVM 4.0 upgrade.

I think the problem may be that the constant has a Size of 0 (if I interpret the IR correctly), could that special case be handled incorrectly by the current code?

Cheers,
Tim

On Sat, Mar 18, 2017, at 10:35 AM, James Molloy via Phabricator wrote:
> jmolloy requested changes to this revision.
> jmolloy added a comment.
> This revision now requires changes to proceed.
> 
> Hi Tim,
> 
> Thanks for looking into this. I'm not 100% certain about your patch though. The intent in this code is that everything gets padded to a multiple of 4 bytes (see PaddingPossible and RequiredPadding above). I'd expect that in the case you describe, RequiredPadding would be != 4, so the loop at line 3095 would padd out the global and avoid putting too-small data in the constant pool.
> 
> Without a reduced testcase (which would be needed to commit the change) I can't offhand see where the problem could actually be.
> 
> Cheers,
> 
> James
> 
> 
> https://reviews.llvm.org/D31116
> 
> 
>