rnk added a comment.

I see two ways forward here:

1. Autoupgrade modules with old datalayout strings by increasing the alignment 
of i128 & co. This will change LLVM IR struct layouts, argument alignments, 
etc. As far as native ABI boundaries are concerned, this should be "more 
correct": Clang explicitly applies `alignstack` attributes to increase the 
alignment of i128 arguments, and adds padding to structs to align i128. As far 
as IR ABI boundaries within LTO are concerned, it is ABI compatible with IR 
modules.
2. Freeze the ABI of the old module during autoupgrade. Replace all struct 
types with equivalent packed structs and explicit padding. Apply explicit 
alignments to all i128 loads and stores. Apply explicit `alignstack(8)` 
attributes to all i128 arguments.

I think 1 is better than 2. The only problem that approach 2 solves is to 
ensure that a non-clang frontend using i128 is ABI compatible with old versions 
of that same frontend (think Rust). Given that most non-clang frontends want 
the bug fix (ABI break), who exactly is asking for this level of IR ABI 
stability? Maybe I'm missing something, but after skimming over this review 
again, I think the existing autoupgrade approach is probably good enough. Can 
we add a release note or something and leave it at that?


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