[PATCH] D92278: [Clang] Don't adjust align for IBM extended double type

Wed Dec 2 23:16:09 PST 2020

qiucf added inline comments.

================
Comment at: clang/test/CodeGen/ppc64le-varargs-f128.c:8

+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -fopenmp-targets=ppc64le -mfloat128 -mabi=ieeelongdouble -mcpu=pwr9 \
----------------
hubert.reinterpretcast wrote:
> MaskRay wrote:
> > qiucf wrote:
> > > MaskRay wrote:
> > > > Generally `%clang` is only used in test/Driver and `%clang_cc1` should be used for tests.
> > > > 
> > > > `%clang_cc1` has a lit substitution rule to find the builtin include directory where stdarg.h can be found.
> > > Thanks! I moved it in rG222da77a
> > rG222da77a82d17cbc6b989779e2ba2bb4904bb672 looks more wrong to me: test/Driver tests how the clang driver passes CC1 options to the frontend. The CodeGen should be tested in this directory.
> > 
> > You probably should split %clang to several %clang_cc1 commands and keep the test here.
> I don't think moving the test such that a "driver test" checks code gen output is the direction @MaskRay was pointing to. Changing the `RUN` line to invoke `-cc1` (using `%clang_cc1`) and leaving the test as a code gen test is what I would have expected.
Ah, thanks for pointing it out. I simplified the splitted commands from `clang -v`. https://reviews.llvm.org/D92544 is created for easier view, if that looks fine, I'll commit it :-)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92278