efriedma added a comment.

In D143386#4110398 <https://reviews.llvm.org/D143386#4110398>, @gchatelet wrote:

>> And the x86 doesn't specify 4-byte alignment.
>
> I thought that specifying it in the `DataLayout` would help getting rid of 
> the special handling in ConstantFold 
> <https://github.com/llvm/llvm-project/blob/2d279c0d95d405098a6148cad6c9a68c92e6df4f/llvm/lib/IR/ConstantFold.cpp#L1062-L1072>.
>  I'm confused now.

That hack basically says "if the datalayout doesn't specify the alignment of 
function pointers, assume function pointers are at least 4-byte aligned".  If 
we add the correct datalayout markings for x86, that should solve the problems 
discussed in D55115 <https://reviews.llvm.org/D55115> with any hacks, 
hopefully.  The relevant functions should have an explicit alignment marking, 
so under an "Fn1" datalayout their alignment would be greater than one.

Currently the x86 backend doesn't actually enforce any alignment for functions 
at -Os/-Oz(no call to setMinFunctionAlignment etc.).  So the correct string for 
x86 is .  If it turns out some minimum is actually necessary for the sake of IR 
optimizations, we'd need to fix that separately.

>> For each target, we should try to handle the LLVM backend at the same time...
>
> I'm not familiar with this part of the codebase as you can tell. Would you 
> have any pointers to the backend part that needs to be updated?
> Shall we try to do one target at a time then?

I haven't really looked at this in a while... but for each target, the backend 
has a method to compute the "canonical" datalayout string.  This is exposed as 
TargetMachine::createDataLayout, but it's computed in each target's 
TargetMachine constructor.  Most ways to invoke the backend enforce this string 
on the module.  (I think clang does this separately so that we can do semantic 
analysis and IR generation even if the relevant backend isn't compiled in.  
Don't remember the details.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143386/new/

https://reviews.llvm.org/D143386

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to