lkail marked an inline comment as done.
lkail added inline comments.
================
Comment at: clang/lib/Basic/Targets/PPC.h:448
+ void setMaxAtomicWidth() override {
+ // For layout on ELF targets, we support up to 16 bytes.
+ if (getTriple().isOSBinFormatELF())
----------------
hubert.reinterpretcast wrote:
> I believe this should be presented more as this override being implemented
> currently only for ELF targets. The current presentation seems to imply more
> design intent for non-ELF targets than there is consensus for.
>
> For example:
> ```
> if (!getTriple().isOSBinFormatELF())
> return PPCTargetInfo::setMaxAtomicWidth();
> ```
It looks a chance to adjust according to arch level to me(Considering we
finally will make xcoff and elf targets consistent here). If you have strong
opinion on this, I'll make a change here.
================
Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:34
// PPC32: @o = global %struct.O zeroinitializer, align 1{{$}}
// PPC64: @o = global %struct.O zeroinitializer, align 8{{$}}
----------------
hubert.reinterpretcast wrote:
> Just noting that GCC increases the alignment even for ppc32:
> ```
> typedef struct A8 { char x[8]; } A8;
> typedef struct A16 { char x[16]; } A16;
> extern char q8[_Alignof(_Atomic(A8))], q8[8]; // okay for GCC targeting ppc32
> extern char q16[_Alignof(_Atomic(A16))], q16[16]; // okay for GCC targeting
> ppc32
> ```
>
> Apparently, the change for i686 in GCC occurred with version 11.
> https://godbolt.org/z/fTTGoqWW1
I'll post another patch to address ppc32 issue.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122377/new/
https://reviews.llvm.org/D122377
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
