tmgross added a comment.

Thank you Craig and Harald for getting back so quick. I suppose that leaves it 
up to what level of `AutoUpgrade` changes would be accepted at a minimum.

@efriedma would you consider the changes suggested by @hvdijk  sufficient under 
any circumstances or would you still insist on fully compatible AutoUpgrade, 
given the above discussion?

In D86310#3226142 <https://reviews.llvm.org/D86310#3226142>, @rnk wrote:

> Who exactly generates GCC-incompatible code, clang, LLVM, or some other 
> frontend? My understanding is that Clang handles most struct layout and 
> alignment concerns in the frontend. The feature I'm not clear on is calling 
> convention lowering, so when i128 is passed in memory, the LLVM data layout 
> controls its alignment. However, I wonder if the `alignstack()` parameter 
> attribute can be used to control this instead from the frontend:
> https://llvm.org/docs/LangRef.html#parameter-attributes

Old question but just to add some more context - LLVM is generating code that 
is incorrect for the linux ABI (16-byte alignment is required, LLVM produces 
8-byte alignment) but the Clang frontend patches this in a way that "mostly 
works". It does not always work, such as in the bug that Herald linked at 
https://bugs.llvm.org/show_bug.cgi?id=50198, which segfaults with the mostt 
recent LLVM versions but is OK with GCC. This is pretty bad because it means 
that any frontend has to provide a workaround just to make LLVM do the mostly 
correct (but still not fully correct) thing.

This came into relevance recently because we are revisiting the issue in Rust. 
I think we are pretty close to providing a hack solution like Clang does, but 
LLVM is objectively wrong here so there are going to be things that just don't 
work correctly for anybody until this gets fixed. There is some thorough 
discussion on our related issue, around this comment 
https://github.com/rust-lang/rust/issues/54341#issuecomment-1064729606.

Note that a fix for this was landed at some point but got reverted, 
https://reviews.llvm.org/D28990. @echristo as you were the reviewer there, do 
you maybe have anything to add about the proposed fix here?


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

https://reviews.llvm.org/D86310

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

Reply via email to