hubert.reinterpretcast added inline comments.

================
Comment at: clang/lib/AST/ASTContext.cpp:2424
+      (T->isSpecificBuiltinType(BuiltinType::LongDouble) &&
+       Target->supportsAIXPowerAlignment()))
     // Don't increase the alignment if an alignment attribute was specified on 
a
----------------
hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > hubert.reinterpretcast wrote:
> > > Does `supportsAIXPowerAlignment` express the condition we want to check 
> > > here? That might be true for an implementation operating with `mac68k` 
> > > alignment rules.
> > Yeah, `supportsAIXPowerAlignment` cannot separate the preferred alignment 
> > of double, long double between `power/natural` and `mac68k` alignment 
> > rules. But I noticed that currently, AIX target on wyvern or XL don't 
> > support `mac68k` , so maybe we should leave further changes to the patch 
> > which is gonna implement `mac68k` alignment rules? The possible solution I 
> > am thinking is we can add checking if the decl has `AlignMac68kAttr` into 
> > query to separate things out.
> > 
> > Another thing is that once we start supporting mac68k alignment rule(if we 
> > will), should we also change the ABI align values as well? (e.g. for 
> > double, it should be 2 instead)
> If the "base state" is AIX `power` alignment for a platform, I suggest that 
> the name be `defaultsToAIXPowerAlignment`.
This last question about the ABI align values is relevant to considerations for 
`natural` alignment support as well. More generally, the question is whether 
the "minimum alignment" of the type in a context subject to alternative 
alignment rules is altered to match said alignment rule. This is observable via 
the diagnostic associated with C++11 alignment specifiers.

The existing behaviour of `mac68k` alignment suggests that the "minimum 
alignment" is context-free.

```
#pragma options align=mac68k
struct Q {
  double x alignas(2);  // expected-error {{less than minimum alignment}}
};
#pragma options align=reset
```

Compiler Explorer link: https://godbolt.org/z/9NM5_-


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79719



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

Reply via email to