[PATCH] D102560: [AIX][clang-repl][test] Mark unsupported pending XCOFF64 integrated-as

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ae529d0db2d: [AIX][clang-repl][test] Mark unsupported 
pending XCOFF64 integrated-as (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102560

Files:
  clang/test/Interpreter/execute.cpp


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,6 +1,6 @@
 // RUN: cat %s | clang-repl | FileCheck %s
 // REQUIRES: host-supports-jit
-// UNSUPPORTED: powerpc64
+// UNSUPPORTED: system-aix
 
 extern "C" int printf(const char *, ...);
 int i = 42;


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,6 +1,6 @@
 // RUN: cat %s | clang-repl | FileCheck %s
 // REQUIRES: host-supports-jit
-// UNSUPPORTED: powerpc64
+// UNSUPPORTED: system-aix
 
 extern "C" int printf(const char *, ...);
 int i = 42;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2761428 , 
@hubert.reinterpretcast wrote:

> If you have some ideas, please let me know. Meanwhile, I am trying out 
> `system-aix` as the "feature" to XFAIL on.

https://reviews.llvm.org/D102560 posted to use `system-aix` so other PPC 
configurations will run the test. Possible further changes is for the code to 
choose the process triple, for the test to possibly specify `-fintegrated-as`, 
and (alternatively) for the system assembler step to be integrated into this 
use scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D102560: [AIX][clang-repl][test] Mark unsupported pending XCOFF64 integrated-as

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: v.g.vassilev, lhames, daltenty, 
jasonliu, jsji.
Herald added a subscriber: steven.zhang.
hubert.reinterpretcast requested review of this revision.
Herald added a project: clang.

This patch replaces the `powerpc64` token with the `system-aix` one in the 
UNSUPPORTED line of a test. The `powerpc64` token was originally added 
temporarily in 71a0609a2b53 
.

If AIX uses integrated-as by default and it works both for 32-bit and 64-bit 
objects, then the issues encountered so far (see comments in D96033 
) would be mostly solved.

As it is, marking the test as expected-to-fail (as opposed to unsupported) on 
AIX might cause more trouble in the form of 32-bit versus 64-bit differences. I 
am not aware of other situations where LIT tests are dependent on whether the 
LLVM build is 64-bit or 32-bit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102560

Files:
  clang/test/Interpreter/execute.cpp


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,6 +1,6 @@
 // RUN: cat %s | clang-repl | FileCheck %s
 // REQUIRES: host-supports-jit
-// UNSUPPORTED: powerpc64
+// UNSUPPORTED: system-aix
 
 extern "C" int printf(const char *, ...);
 int i = 42;


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -1,6 +1,6 @@
 // RUN: cat %s | clang-repl | FileCheck %s
 // REQUIRES: host-supports-jit
-// UNSUPPORTED: powerpc64
+// UNSUPPORTED: system-aix
 
 extern "C" int printf(const char *, ...);
 int i = 42;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Once I add `-Xcc -fintegrated-as`, we get:

  $ cat /home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/clang-repl -Xcc 
-fintegrated-as
  fatal error: error in backend: 64-bit XCOFF object files are not supported 
yet.
  clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 

I am not sure if there's something to avoid this other than to XFAIL the test 
somehow while the 64-bit XCOFF integrated-as capability is still pending.
If you have some ideas, please let me know. Meanwhile, I am trying out 
`system-aix` as the "feature" to XFAIL on.

I still think the switch to the use the process triple in 
https://reviews.llvm.org/D96033#2759808 is perhaps correct (so even if it does 
not help currently for the configuration we have running, it could be 
worthwhile to commit).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@v.g.vassilev, thanks for working with me on this. I understand it is difficult 
to handle issues that appear on platforms and build configurations one does not 
have set up.

I've added `-Xcc -v` and the output is below. It seems it has to do with the 
implicit `-fno-integrated-as` currently used with AIX. I'll paste the result 
with `-Xcc -fintegrated-as` in my next comment.

  $ cat /home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/clang-repl -Xcc -v
  clang version 13.0.0
  Target: powerpc64-ibm-aix7.2.0.0
  Thread model: posix
  InstalledDir:
   "" -cc1 -triple powerpc64-ibm-aix7.2.0.0 -S -disable-free -main-file-name 
"<<< inputs >>>" -mrelocation-model pic -pic-level 2 -mframe-pointer=all 
-fmath-errno -fno-rounding-math -fno-verbose-asm -no-integrated-as -target-cpu 
pwr7 -mfloat-abi hard -mllvm -treat-scalable-fixed-error-as-warning 
-gstrict-dwarf -gno-column-info -debugger-tuning=dbx -v -fdata-sections 
-fcoverage-compilation-dir=/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build 
-resource-dir lib/clang/13.0.0 -internal-isystem lib/clang/13.0.0/include 
-internal-isystem /usr/include -fdeprecated-macro -fno-dwarf-directory-asm 
-fdebug-compilation-dir=/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build 
-ferror-limit 19 -fno-signed-char -fno-use-cxa-atexit -fgnuc-version=4.2.1 
-fcxx-exceptions -fexceptions -fcolor-diagnostics -fxl-pragma-pack -o 
"/tmp/hstong/auto.2021W19/<<< inputs >>>-e534ce.s" -x c++ "<<< inputs >>>"
   "/usr/bin/as" -a64 -many -o "<<< inputs >>>.o" "/tmp/hstong/auto.2021W19/<<< 
inputs >>>-e534ce.s"
  clang-repl: Driver initialization failed. Incremental mode for action is not 
supported


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2759808 , @v.g.vassilev 
wrote:

> Hi @hubert.reinterpretcast,
>
> Would you mind testing this patch:

Does the test try to generate native object files in some way? There is 
functionality (with some limitations) for that under 32-bit AIX; however, we're 
running a 64-bit build and we don't have integrated-as capability for that at 
this time. This is what I'm seeing:

   TEST 'Clang :: Interpreter/execute.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   cat 
/home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/clang-repl | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/FileCheck 
/home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp
  --
  Exit Code: 2
  
  Command Output (stderr):
  --
  clang-repl: Driver initialization failed. Incremental mode for action is not 
supported
  FileCheck error: '' is empty.
  FileCheck command line:  
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/FileCheck 
/home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp
  
  --
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2759808 , @v.g.vassilev 
wrote:

> Hi @hubert.reinterpretcast,
>
> Would you mind testing this patch:

Running it now. I've applied the first diff here to the base of my previously 
reported result to minimize build time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2758189 , @lhames wrote:

> Ok, looks like the JIT is getting the layout right, but clang-repl is 
> constructing a module with an little-endian layout for some reason.

`clang-repl` is generating a module consistent with 
`LLVM_DEFAULT_TARGET_TRIPLE:STRING=powerpc64le-linux-gnu`.
So it seems this test might need some way of telling `clang-repl` to use the 
host triple.

> In the mean time I have conditionally disabled this test on ppc64 in 
> 71a0609a2b 
> .

That will help our builds; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2757342 , @lhames wrote:

> Hi Hubert,
>
> Could you apply the following patch and let me know the output from the 
> failing test? I'm trying to work out whether the JIT is getting the triple or 
> the data layout wrong.



   TEST 'Clang :: Interpreter/execute.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   cat 
/home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/clang-repl | 
/home/hstong/.Nrtphome/.Liodine/llcrossbld/dev/build/bin/FileCheck 
/home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  triple: powerpc64-ibm-aix7.2.0.0
  datalayout: E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  error: Added modules have incompatible data layouts: 
e-m:e-i64:64-n32:64-S128-v256:256:256-v512:512:512 (module) vs 
E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512 (jit)
  /home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp:7:11: 
error: CHECK: expected string not found in input
  // CHECK: i = 42
^
  :1:1: note: scanning from here
  clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> 
  ^
  :1:9: note: possible intended match here
  clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> 
  ^
  
  Input file: 
  Check file: /home/hstong/.Liodine/llvmproj/clang/test/Interpreter/execute.cpp
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl>  
  check:7'0 
X
 error: no match found
  check:7'1 ?   

  possible intended match
  >>
  
  --
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2757342 , @lhames wrote:

> Hi Hubert,
>
> Could you apply the following patch and let me know the output from the 
> failing test? I'm trying to work out whether the JIT is getting the triple or 
> the data layout wrong.

I've started the build. My tree was a bit stale, so it might not be the fastest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D96033#2757179 , @v.g.vassilev 
wrote:

> In D96033#2757107 , @v.g.vassilev 
> wrote:
>
>> In D96033#2757077 , 
>> @hubert.reinterpretcast wrote:
>>
>>> @v.g.vassilev, the test does not appear to be appropriately set up for 
>>> builds that default to a non-native target:
>>> https://lab.llvm.org/staging/#/builders/126/builds/371/steps/5/logs/FAIL__Clang__execute_cpp
>>>
>>> Can you fix or revert until there is a fix?
>>
>> Apologies. Do you know if `REQUIRES: native` is sufficient to fix (trying to 
>> avoid churn)?

A speculative application of the above probably helps and would be harmless (I 
think).

> @lhames and I are working on a patch. We do not have easy access to such 
> machines. Would you mind testing it on the bot?

I have a local build I can apply a patch to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@v.g.vassilev, the test does not appear to be appropriately set up for builds 
that default to a non-native target:
https://lab.llvm.org/staging/#/builders/126/builds/371/steps/5/logs/FAIL__Clang__execute_cpp

Can you fix or revert until there is a fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96033

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

[PATCH] D102064: Parse vector bool when stdbool.h and altivec.h are included

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with comment.




Comment at: clang/test/Parser/altivec-zvector-bool.c:20-23
+__vector _Bool char Bc;
+__vector _Bool short Bsh;
+__vector _Bool short int Bshi;
+__vector _Bool int Bi;

I think having these as declarations of the names used above is beneficial 
because it confirms that the types formed using the `bool` spelling are 
compatible with the same using the `_Bool` spelling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102064

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

[PATCH] D102064: Parse vector bool when stdbool.h and altivec.h are included

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:7331-7332
 }
-if (Next.getIdentifierInfo() == Ident_bool) {
+if ((Next.getIdentifierInfo() == Ident_bool) ||
+Next.getIdentifierInfo() == Ident_Bool) {
   Tok.setKind(tok::kw___vector);

Minor nit: remove (inconsistently applied) extra parentheses.



Comment at: clang/lib/Parse/Parser.cpp:515
 
+  if (getLangOpts().AltiVec)
+Ident_Bool = ().get("_Bool");

No need for an extra `if` block. Just need to figure out which block above this 
should go in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102064

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

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/cmake/config-ix.cmake:285
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)

jsji wrote:
> Maybe add a warning here about we are forcing it to 0?
As discussed off-list: For a platform where Clang and LLVM is relatively new, I 
don't think this leads to a loss of functionality at the level where a warning 
is warranted. The autodetection is going to set this to 0 on some platforms 
anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

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

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rGbdc4ec04d42a: [AIX] Avoid use of mtim.tv_nsec member of stat 
structure on AIX (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

Files:
  llvm/cmake/config-ix.cmake


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D101323#2717925 , @jsji wrote:

> Have we considered workaround this in `llvm/lib/Support/Unix/Path.inc` 
> instead?

Yes. I think this is a more general fix that would automatically apply in other 
situations where code should be checking for this macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

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

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: daltenty, Xiangling_L, jsji.
Herald added a subscriber: mgorny.
hubert.reinterpretcast requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The value observed for the `mtim.tv_nsec` member is erroneous in some AIX 
environments. Avoid using this member by forcing 
`HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC` to `0`.

This resolves "mtime changed" errors such as the one 
http://lab.llvm.org:8014/#/builders/126/builds/330/steps/5/logs/FAIL__Clang__test_c
 has.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101323

Files:
  llvm/cmake/config-ix.cmake


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99483: [clang][AIX] Define __STDC_NO_ATOMICS__ for c11 and above

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:727
+// The AIX library implementation of C11 atomics is not yet complete.
+if (Opts.C11 || Opts.C17 || Opts.C2x) {
+  Builder.defineMacro("__STDC_NO_ATOMICS__");

Checking `Opts.C11` might be sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99483

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

[PATCH] D98554: Save strings for CC_PRINT env vars

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.




Comment at: clang/lib/Driver/Driver.cpp:4042
 
-  if (!CCPrintStatReportFilename) {
+  if (CCPrintStatReportFilename.empty()) {
 using namespace llvm;

Just noting that this means having the environment variable set (but empty) 
will now "work" instead of generating an error.


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

https://reviews.llvm.org/D98554

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

[PATCH] D98660: [AIX][XCOFF] Fixed the test case which failed at aix OS because enable -mignore-xcoff-visibility by default.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM (with the equal sign form as @daltenty requested if it works).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98660

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:2061-2062
 TypeKind.FLOAT128 = TypeKind(30)
 TypeKind.HALF = TypeKind(31)
+TypeKind.IBM128 = TypeKind(32)
 TypeKind.COMPLEX = TypeKind(100)

This looks suspiciously like the result of this file having not been maintained 
for the additions of:
```
  CXType_Float16 = 32,
  CXType_ShortAccum = 33,
  CXType_Accum = 34,
  CXType_LongAccum = 35,
  CXType_UShortAccum = 36,
  CXType_UAccum = 37,
  CXType_ULongAccum = 38,
  CXType_BFloat16 = 39,
```

What test coverage fails if the addition of `TypeKind.IBM128` is omitted from 
this patch?



Comment at: clang/include/clang-c/Index.h:3283
   CXType_FirstBuiltin = CXType_Void,
   CXType_LastBuiltin = CXType_BFloat16,
 

Looks like `CXType_Ibm128` is the "last" built-in type now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > jwakely wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > qiucf wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > > Seems clang is also different from GCC under 
> > > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > > the same for GCC but not for clang.
> > > > > > Have you checked whether the new libstdc++ for which this support 
> > > > > > is being added needs the GCC behaviour to work properly?
> > > > > > 
> > > > > > The GCC behaviour allows the following to be compiled without 
> > > > > > introducing novel overload resolution tiebreakers:
> > > > > > ```
> > > > > > void f(__float128);
> > > > > > void f(__ibm128);
> > > > > > void f(int);
> > > > > > 
> > > > > > long double ld;
> > > > > > 
> > > > > > int main() { f(ld); }
> > > > > > ```
> > > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > > 
> > > > > ```
> > > > > std::cout << "long double:\n";
> > > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > > 
> > > > > std::cout << "__float128:\n";
> > > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > > 
> > > > > std::cout << "__ibm128:\n";
> > > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > > ```
> > > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > > sure you have a sufficiently new libstdc++?
> > > > @jwakely, are the overload resolution errors expected? 
> > > 
> > > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > > happens to be the same type as `long double` (due to a 
> > > `-mabi=ieeelongdouble` option).
> > Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> > diverge from GCC.
> > 
> > For example, Clang reports the call below as ambiguous even with 
> > `-mabi=ieeelongdouble`:
> > ```
> > void f(double);
> > void f(long double);
> > 
> > void g(__float128 f128) { f(f128); }
> > ```
> > 
> > https://godbolt.org/z/dhYEKa
> > 
> > Insofar as the user experience goes, is this lack of iostreams support for 
> > `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> > intended design of libstdc++?
> The lack of iostreams support for `__float128` is the intended design.
> 
> On power we support `float`, `double` and three types of `long double`. If 
> `__float128` is a distinct type from all those `long double` types it won't 
> work.
> 
> GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it 
> is the same as one of the `long double` types.
Okay, it sounds like the different treatment Clang has does not really 
interfere with recommended usage insofar as libstdc++ iostreams (and hopefully 
this extends to the rest of the library).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > qiucf wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? 
> > > > > I saw `__float128` and `long double` are the same for GCC but not for 
> > > > > clang.
> > > > Have you checked whether the new libstdc++ for which this support is 
> > > > being added needs the GCC behaviour to work properly?
> > > > 
> > > > The GCC behaviour allows the following to be compiled without 
> > > > introducing novel overload resolution tiebreakers:
> > > > ```
> > > > void f(__float128);
> > > > void f(__ibm128);
> > > > void f(int);
> > > > 
> > > > long double ld;
> > > > 
> > > > int main() { f(ld); }
> > > > ```
> > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > 
> > > ```
> > > std::cout << "long double:\n";
> > > std::cout << std::numeric_limits::max() << std::endl;
> > > std::cout << std::numeric_limits::min() << std::endl;
> > > 
> > > std::cout << "__float128:\n";
> > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > 
> > > std::cout << "__ibm128:\n";
> > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > ```
> > @jwakely, are the overload resolution errors expected? @qiucf, are you sure 
> > you have a sufficiently new libstdc++?
> > @jwakely, are the overload resolution errors expected? 
> 
> Yes. Iostreams support `long double` but not `__float128`, unless that 
> happens to be the same type as `long double` (due to a `-mabi=ieeelongdouble` 
> option).
Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
diverge from GCC.

For example, Clang reports the call below as ambiguous even with 
`-mabi=ieeelongdouble`:
```
void f(double);
void f(long double);

void g(__float128 f128) { f(f128); }
```

https://godbolt.org/z/dhYEKa

Insofar as the user experience goes, is this lack of iostreams support for 
`__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
intended design of libstdc++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

qiucf wrote:
> hubert.reinterpretcast wrote:
> > qiucf wrote:
> > > hubert.reinterpretcast wrote:
> > > > Not sure what the best method is to implement this, but `long double` 
> > > > and `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is 
> > > > in effect.
> > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I 
> > > saw `__float128` and `long double` are the same for GCC but not for clang.
> > Have you checked whether the new libstdc++ for which this support is being 
> > added needs the GCC behaviour to work properly?
> > 
> > The GCC behaviour allows the following to be compiled without introducing 
> > novel overload resolution tiebreakers:
> > ```
> > void f(__float128);
> > void f(__ibm128);
> > void f(int);
> > 
> > long double ld;
> > 
> > int main() { f(ld); }
> > ```
> As I saw both GCC and clang have error for ambiguous `operator<<` for:
> 
> ```
> std::cout << "long double:\n";
> std::cout << std::numeric_limits::max() << std::endl;
> std::cout << std::numeric_limits::min() << std::endl;
> 
> std::cout << "__float128:\n";
> std::cout << std::numeric_limits<__float128>::max() << std::endl;
> std::cout << std::numeric_limits<__float128>::min() << std::endl;
> 
> std::cout << "__ibm128:\n";
> std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> ```
@jwakely, are the overload resolution errors expected? @qiucf, are you sure you 
have a sufficiently new libstdc++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D98554: Save strings for CC_PRINT env vars

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:160
   /// The file to log CC_PRINT_PROC_STAT_FILE output to, if enabled.
-  const char *CCPrintStatReportFilename;
+  std::string CCPrintStatReportFilename;
 

I'm seeing code left unchanged like:
```
TheDriver.CCPrintOptionsFilename = ::getenv("CC_PRINT_OPTIONS_FILE");
```

Assigning to a `std::string` from a null `char *` is known to cause things like
```
Segmentation fault
```




Comment at: clang/lib/Driver/Compilation.cpp:173
 // output stream.
-if (getDriver().CCPrintOptions && getDriver().CCPrintOptionsFilename) {
+if (getDriver().CCPrintOptions && 
!getDriver().CCPrintOptionsFilename.empty()) {
   std::error_code EC;

Please update for the clang-format suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98554

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

[PATCH] D98552: [NFC] Adjust SmallVector.h header to workaround XL build compiler issue

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

Retained `using` declarations look fine. LGTM.


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

https://reviews.llvm.org/D98552

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

[PATCH] D98552: [NFC] Adjust SmallVector.h header to workaround XL build compiler issue

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

Thanks @Xiangling_L and also to the other reviewers for the great comments. 
This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98552

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

[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

sbc100 wrote:
> hubert.reinterpretcast wrote:
> > curdeius wrote:
> > > hubert.reinterpretcast wrote:
> > > > curdeius wrote:
> > > > > curdeius wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > sbc100 wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > This seems to be assuming that the underlying C library's 
> > > > > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// 
> > > > > > > > > define `alignof` and `alignas` as macros.
> > > > > > > > Should I just remove this `#include` then?
> > > > > > > The idea would be to //add// a `stdalign.h` alongside this header 
> > > > > > > that doesn't `#include_next` the underlying C library's 
> > > > > > > `stdalign.h`.
> > > > > > I'm not sure if that should be the solution. At least gcc's 
> > > > > > libstdc++ assumes that `stdalign.h` is C++-compatbile (cf. 
> > > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > > > > 
> > > > > > Clang provides a compatible header: 
> > > > > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > > > > Gcc too of course: 
> > > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > > > > 
> > > > > > MSVC's STL on the other hand, doesn't include `` 
> > > > > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > > > > 
> > > > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > > > non-friendly `stdalign.h`?
> > > > > FYI, musl is also C++ friendly: 
> > > > > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> > > > >>! Quote:
> > > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > > non-friendly stdalign.h?
> > > > 
> > > > The one GCC provides disagrees with the interpretation I gave of which 
> > > > macros should be present. The one that Clang provides //does// match my 
> > > > interpretation. It seems the GCC one is non-friendly (albeit a 
> > > > different form of non-friendly than the one I opened with).
> > > Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't 
> > > be defined in this case, right?
> > > In this case, I guess we won't avoid having `stdalign.h` as you had 
> > > suggested.
> > > And indeed the test fails with gcc:
> > > ```
> > > bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
> > > --param=std=c++17 --param=cxx_under_test=`which g++`
> > > ...
> > > libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: 
> > > error: #error __alignas_is_defined not defined
> > >21 | #error __alignas_is_defined not defined
> > >   |  ^
> > > ```
> > > 
> > > That's unfortunately a configuration which is not tested in the CI.
> > Yes, it looks like adding a `stdalign.h` for libc++ is needed to reliably 
> > get `__alignas_is_defined` and `__alignof_is_defined`.
> Alternatively, could we just defined them if they are not already and skip 
> including `stdalign.h` here?
`stdalign.h` is a header defined in C++17 like `stdbool.h` is one. libc++ has a 
`stdbool.h`, so I think having a `stdalign.h` makes sense (since we know that 
`stdalign.h` in some environments does not do what we want).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

curdeius wrote:
> hubert.reinterpretcast wrote:
> > curdeius wrote:
> > > curdeius wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > sbc100 wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > This seems to be assuming that the underlying C library's 
> > > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// define 
> > > > > > > `alignof` and `alignas` as macros.
> > > > > > Should I just remove this `#include` then?
> > > > > The idea would be to //add// a `stdalign.h` alongside this header 
> > > > > that doesn't `#include_next` the underlying C library's `stdalign.h`.
> > > > I'm not sure if that should be the solution. At least gcc's libstdc++ 
> > > > assumes that `stdalign.h` is C++-compatbile (cf. 
> > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > > 
> > > > Clang provides a compatible header: 
> > > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > > Gcc too of course: 
> > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > > 
> > > > MSVC's STL on the other hand, doesn't include `` 
> > > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > > 
> > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > non-friendly `stdalign.h`?
> > > FYI, musl is also C++ friendly: 
> > > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> > >>! Quote:
> > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > non-friendly stdalign.h?
> > 
> > The one GCC provides disagrees with the interpretation I gave of which 
> > macros should be present. The one that Clang provides //does// match my 
> > interpretation. It seems the GCC one is non-friendly (albeit a different 
> > form of non-friendly than the one I opened with).
> Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't be 
> defined in this case, right?
> In this case, I guess we won't avoid having `stdalign.h` as you had suggested.
> And indeed the test fails with gcc:
> ```
> bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
> --param=std=c++17 --param=cxx_under_test=`which g++`
> ...
> libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: error: 
> #error __alignas_is_defined not defined
>21 | #error __alignas_is_defined not defined
>   |  ^
> ```
> 
> That's unfortunately a configuration which is not tested in the CI.
Yes, it looks like adding a `stdalign.h` for libc++ is needed to reliably get 
`__alignas_is_defined` and `__alignof_is_defined`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

curdeius wrote:
> curdeius wrote:
> > hubert.reinterpretcast wrote:
> > > sbc100 wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > This seems to be assuming that the underlying C library's 
> > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// define 
> > > > > `alignof` and `alignas` as macros.
> > > > Should I just remove this `#include` then?
> > > The idea would be to //add// a `stdalign.h` alongside this header that 
> > > doesn't `#include_next` the underlying C library's `stdalign.h`.
> > I'm not sure if that should be the solution. At least gcc's libstdc++ 
> > assumes that `stdalign.h` is C++-compatbile (cf. 
> > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > 
> > Clang provides a compatible header: 
> > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > Gcc too of course: 
> > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > 
> > MSVC's STL on the other hand, doesn't include `` 
> > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > 
> > @hubert.reinterpretcast, are you aware of an environment which has 
> > non-friendly `stdalign.h`?
> FYI, musl is also C++ friendly: 
> https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
>>! Quote:
> @hubert.reinterpretcast, are you aware of an environment which has 
> non-friendly stdalign.h?

The one GCC provides disagrees with the interpretation I gave of which macros 
should be present. The one that Clang provides //does// match my 
interpretation. It seems the GCC one is non-friendly (albeit a different form 
of non-friendly than the one I opened with).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D98265: [NFC] Use llvm::SmallVector to workaround XL compiler problem on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98265

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

[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

sbc100 wrote:
> hubert.reinterpretcast wrote:
> > This seems to be assuming that the underlying C library's `stdalign.h` is 
> > C++ friendly. A C11 `stdalign.h` //does// define `alignof` and `alignas` as 
> > macros.
> Should I just remove this `#include` then?
The idea would be to //add// a `stdalign.h` alongside this header that doesn't 
`#include_next` the underlying C library's `stdalign.h`.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif

sbc100 wrote:
> hubert.reinterpretcast wrote:
> > sbc100 wrote:
> > > ldionne wrote:
> > > > I'm not seeing `__alignof_is_defined` anywhere in the spec?
> > > Removed
> > Seems like a defect in the old standard. The prose doesn't match the 
> > synopsis. `__alignof_is_defined` is a macro in C11's `stdalign.h` (and so 
> > is `alignof`). That the C++ committee did not intend for an `alignof` macro 
> > can probably be assumed. I suspect the lack of an `__alignof_is_defined` 
> > macro was also unintended.
> So should I add back the check for `__alignof_is_defined`?
I think so (in addition to also checking that `alignof` is not defined as a 
macro). I think the patch needs to be confirmed again either way by the libc++ 
approvers though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

This seems to be assuming that the underlying C library's `stdalign.h` is C++ 
friendly. A C11 `stdalign.h` //does// define `alignof` and `alignas` as macros.



Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif

sbc100 wrote:
> ldionne wrote:
> > I'm not seeing `__alignof_is_defined` anywhere in the spec?
> Removed
Seems like a defect in the old standard. The prose doesn't match the synopsis. 
`__alignof_is_defined` is a macro in C11's `stdalign.h` (and so is `alignof`). 
That the C++ committee did not intend for an `alignof` macro can probably be 
assumed. I suspect the lack of an `__alignof_is_defined` macro was also 
unintended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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

[PATCH] D46443: Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.
Herald added a subscriber: wingo.

The resolution of LWG 2828 
 means that `` 
exists in C++11 and C++14. The rationale from 
https://reviews.llvm.org/D96786#2566110 can be taken as saying that adding this 
header for C++11 conformance is reasonable.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46443

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

[PATCH] D95822: [FE] Manipulate the first byte of guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM. I think there would be more concern if the load/store actions were 
previously symmetric, but this patch is actually needed for the symmetry. 
Nevertheless, please wait a bit before committing this (since this is a change 
for other platforms as well).


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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE] Manipulate the first byte of guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/global-init.cpp:84
   // CHECK-NEXT: store i32 [[CALL]], i32* @_ZN5test41xE
-  // CHECK-NEXT: store i64 1, i64* @_ZGVN5test41xE
+  // CHECK-NEXT: store i8 1, i8* bitcast (i64* @_ZGVN5test41xE to i8*)
   __attribute__((weak)) int x = foo();

Xiangling_L wrote:
> abhina.sreeskantharajan wrote:
> > nit: the lines above omit the end of the line. It's probably better to be 
> > consistent
> > 
> > 
> It didn't omit the end of the line. It is because before my patch, there was 
> no casting.
Hi @Xiangling_L, I think Abhina meant the lines above in this file. The `load` 
has a `bitcast` too (and stops after the guard variable name).


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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE] Manipulate the first byte of guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: guard.patch:1
+From 9c7d0afec1e634cdeaf3a2ca4273f5da07f6f946 Mon Sep 17 00:00:00 2001
+From: Xiangling Liao 

Looks like this file got added by accident.


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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/aix-static-init.cpp:143
 // CHECK: init.check:
-// CHECK:   %1 = call i32 @__cxa_guard_acquire(i64* 
@_ZGVZN5test41fEvE11staticLocal)
+// CHECK:   %1 = call i32 @__cxa_guard_acquire(i8* 
@_ZGVZN5test41fEvE11staticLocal)
 // CHECK:   %tobool = icmp ne i32 %1, 0

What actually does the definition of `_ZGVZN5test41fEvE11staticLocal` look 
like? The test seems to be missing something pertinent for the change being 
proposed here. I will note that the symbol is 8-bytes (and aligned accordingly) 
from XL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95822

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

[PATCH] D95822: [FE][AIX] Use i8 as guard variable type in both load and store operation

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp:193
 // CHECK:   %1 = call i32 @atexit(void ()* @__dtor__ZN5test12t1IiEE)
-// CHECK:   store i64 1, i64* @_ZGVN5test12t1IiEE
 // CHECK:   br label %init.end

Okay, so the store here is wrong, but it's got nothing to do with AIX.

The ABI doc (http://itanium-cxx-abi.github.io/cxx-abi/abi.html#once-ctor) says:
> the first byte (i.e., the byte with lowest address)

So the bug here seems to be that the Itanium ABI implementation in Clang is 
incorrect for big-endian systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95822

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

[PATCH] D95702: [AIX] Improve option processing for mabi=vec-extabi and mabi=vec=defaul

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95702

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

[Diffusion] rGcaaaebcde462: [AIX] Actually push back "-mabi=vec-extabi" when option is on.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.

BRANCHES
  main

/clang/lib/Driver/ToolChains/Clang.cpp:4672-4678 This seems to be saying that 
`-maltivec` rather actively implies `-mabi=vec-default` (on AIX) even in cases 
where `-maltivec` really makes no difference. I suggest just removing this. If 
kept, this needs more code comments and should be moved below to become a check 
only in the case where neither `-mabi=vec-extabi` nor `-mabi=vec-default` was 
present.
/clang/lib/Driver/ToolChains/Clang.cpp:4687 This `if` can be replaced with 
`else`. In this case, although the body of the preceding `if` "exits", I think 
using `else` is reasonable since the mechanism used is not a standard library 
function or jump statement. Any qualms about this can also be alleviated by 
reversing the order of the check.

Users:
  ZarkoCA (Author)

https://reviews.llvm.org/rGcaaaebcde462

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

[Diffusion] rGcaaaebcde462: [AIX] Actually push back "-mabi=vec-extabi" when option is on.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added subscribers: ZarkoCA, hubert.reinterpretcast, 
cfe-commits.

BRANCHES
  main

Users:
  ZarkoCA (Author)

https://reviews.llvm.org/rGcaaaebcde462

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

[PATCH] D63852: [Clang] Move assembler into a separate file

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D63852#2517500 , @aykevl wrote:

> or maybe I've put `AssemblerInvocation` in the wrong directory/library.

This seems to be a good design question. I think a lot of people consider 
`-cc1` functionality to be the "frontend". I am not sure that goes for `-cc1as` 
functionality. This might even be a new library in a sense (a 
libclangAssemblyFrontend).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63852

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

[PATCH] D63852: [Clang] Move assembler into a separate file

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

@aykevl, please check http://lab.llvm.org:8011/#/builders/57/builds/3704: it 
seems you are missing a change to `clang/lib/Frontend/CMakeLists.txt` to update 
the `LLVM_LINK_COMPONENTS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63852

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

[PATCH] D94986: Remove requirement for -maltivec to be used when using -mabi=vec-extabi or -mabi=vec-default when not using vector code

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4671
+  D.Diag(diag::err_aix_default_altivec_abi);
   }
 

Just to confirm, because the patch description confuses me a bit on this: The 
intent is to not complain on `-mabi=vec-default` or `-mabi=vec-extabi` at all 
on AIX, right? That is, `-mabi=vec-extabi` is one way to prepare non-vector 
code for eventual compatibility purposes. @ZarkoCA, I suggest updating the 
patch description (and eventual commit message).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94986

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

[PATCH] D94986: Remove requirement for -maltivec to be used when using -mabi=vec-extabi or -mabi=vec-default when not using vector code

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4669
   << A->getSpelling() << RawTriple.str();
-if (!Args.hasArg(options::OPT_maltivec))
-  D.Diag(diag::err_aix_altivec);
+if (Args.hasArg(options::OPT_mabi_EQ_vec_default))
+  D.Diag(diag::err_aix_default_altivec_abi);

Only generate the message when the option is active.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94986

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

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6ffe4d76fbf: [clang] Fix message text for `-Wpointer-sign` 
to account for plain char (authored by hubert.reinterpretcast).

Changed prior to commit:
  https://reviews.llvm.org/D93999?vs=315648=315956#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/incompatible-sign.c
  clang/test/Sema/incompatible-sign.cpp
  clang/test/SemaObjC/objc-cf-audited-warning.m

Index: clang/test/SemaObjC/objc-cf-audited-warning.m
===
--- clang/test/SemaObjC/objc-cf-audited-warning.m
+++ clang/test/SemaObjC/objc-cf-audited-warning.m
@@ -20,5 +20,5 @@
 
 void saveImageToJPG(const char *filename)
 {
-CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types with different sign}}
+CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
Index: clang/test/Sema/incompatible-sign.cpp
===
--- clang/test/Sema/incompatible-sign.cpp
+++ clang/test/Sema/incompatible-sign.cpp
@@ -4,11 +4,11 @@
 void plainToSigned() {
   extern char c;
   signed char *p;
-  p =  // expected-error {{converts between pointers to integer types with different sign}}
+  p =  // expected-error {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
 
 void unsignedToPlain() {
   extern unsigned char uc;
   char *p;
-  p =  // expected-error {{converts between pointers to integer types with different sign}}
+  p =  // expected-error {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
Index: clang/test/Sema/incompatible-sign.c
===
--- clang/test/Sema/incompatible-sign.c
+++ clang/test/Sema/incompatible-sign.c
@@ -6,18 +6,18 @@
 
 signed char *plainCharToSignedChar(signed char *arg) { // expected-note{{passing argument to parameter}}
   extern char c;
-  signed char *p =  // expected-warning {{converts between pointers to integer types with different sign}}
-  struct { signed char *p; } s = {  }; // expected-warning {{converts between pointers to integer types with different sign}}
-  p =  // expected-warning {{converts between pointers to integer types with different sign}}
-  plainCharToSignedChar(); // expected-warning {{converts between pointers to integer types with different sign}}
-  return  // expected-warning {{converts between pointers to integer types with different sign}}
+  signed char *p =  // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  struct { signed char *p; } s = {  }; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  p =  // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  plainCharToSignedChar(); // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  return  // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
 }
 
 char *unsignedCharToPlainChar(char *arg) { // expected-note{{passing argument to parameter}}
   extern unsigned char uc[];
-  char *p = uc; // expected-warning {{converts between pointers to integer types with different sign}}
-  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types with different sign}}
-  p = uc; // expected-warning {{converts between pointers to integer types with different sign}}
-  unsignedCharToPlainChar(uc); // expected-warning {{converts between pointers to integer types with different sign}}
-  return uc; // expected-warning {{converts between pointers to integer types with different sign}}
+  char *p = uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types where one is of the unique plain 'char' type and the other is not}}
+ 

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast updated this revision to Diff 315648.
hubert.reinterpretcast added a comment.

- Address review: Mention plain char only when it appears


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/incompatible-sign.c
  clang/test/Sema/incompatible-sign.cpp
  clang/test/SemaObjC/objc-cf-audited-warning.m

Index: clang/test/SemaObjC/objc-cf-audited-warning.m
===
--- clang/test/SemaObjC/objc-cf-audited-warning.m
+++ clang/test/SemaObjC/objc-cf-audited-warning.m
@@ -20,5 +20,5 @@
 
 void saveImageToJPG(const char *filename)
 {
-CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types with different sign}}
+CFURLRef url = CFURLCreateFromFileSystemRepresentation(kCFAllocatorDefault, filename, 10, 0); // expected-warning {{passing 'const char *' to parameter of type 'const UInt8 *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain char type and the other is not}}
 }
Index: clang/test/Sema/incompatible-sign.cpp
===
--- /dev/null
+++ clang/test/Sema/incompatible-sign.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -fno-signed-char
+
+void plainToSigned() {
+  extern char c;
+  signed char *p;
+  p =  // expected-error {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+}
+
+void unsignedToPlain() {
+  extern unsigned char uc;
+  char *p;
+  p =  // expected-error {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+}
Index: clang/test/Sema/incompatible-sign.c
===
--- clang/test/Sema/incompatible-sign.c
+++ clang/test/Sema/incompatible-sign.c
@@ -1,5 +1,23 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -fno-signed-char
 
 int a(int* x); // expected-note{{passing argument to parameter 'x' here}}
 int b(unsigned* y) { return a(y); } // expected-warning {{passing 'unsigned int *' to parameter of type 'int *' converts between pointers to integer types with different sign}}
 
+signed char *plainCharToSignedChar(signed char *arg) { // expected-note{{passing argument to parameter}}
+  extern char c;
+  signed char *p =  // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  struct { signed char *p; } s = {  }; // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  p =  // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  plainCharToSignedChar(); // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  return  // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+}
+
+char *unsignedCharToPlainChar(char *arg) { // expected-note{{passing argument to parameter}}
+  extern unsigned char uc[];
+  char *p = uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  (void) (char *[]){ [42] = uc }; // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  p = uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  unsignedCharToPlainChar(uc); // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+  return uc; // expected-warning {{converts between pointers to integer types where one is of the unique plain char type and the other is not}}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15962,6 +15962,16 @@
   else
 FDiag << FirstType << SecondType << Action << SrcExpr->getSourceRange();
 
+  if (DiagKind == diag::ext_typecheck_convert_incompatible_pointer_sign ||
+  DiagKind == diag::err_typecheck_convert_incompatible_pointer_sign) {
+const auto isPlainChar = [](const clang::Type *Type) {
+  return 

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7783-7784
   "|%diff{casting $ to type $|casting between types}0,1}2"
-  " converts between pointers to integer types with different sign">,
+  " converts between pointers to integer types that differ by"
+  " signed/unsigned/plain variation">,
   InGroup>;

aaron.ballman wrote:
> I fee like `that differ by signed/unsigned/plain variation` is a bit hard for 
> users to understand and maybe we want to spell it out a bit more explicitly. 
> I took a stab at a more wordy diagnostic that I think is easier to 
> understand, but if you have other ideas, I'm not tied to my wording. WDYT?
> 
> (Same suggestion applies below -- though we may want to switch to 
> `ext_typecheck_convert_incompatible_pointer_sign.Text` below rather than 
> spelling all this out manually.)
I think the "other excludes sign information" wording makes this sound like a 
portability diagnostic. I was going for wording that retains the same 
"seriousness" for the plain char versus signed/unsigned char case as for the 
signed char versus unsigned char case.

Would "where one is of the unique plain char type and the other is not" work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

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

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Ping, @aaron.ballman?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93999

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

[PATCH] D93095: Introduce -Wreserved-identifier

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/reserved-identifier.cpp:40
+  return foo__bar(); // no-warning
+}

serge-sans-paille wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > You should also have some tests for:
> > > ```
> > > template 
> > > void _Foobar(); // Even though it's not instantiated, it's still reserved.
> > > 
> > > template  // Reserved
> > > void whatever();
> > > 
> > > void func() {
> > >   int array[10];
> > >   for (auto _A : array) // Reserved
> > > ;
> > > }
> > > 
> > > class _C { // Reserved
> > > public:
> > >   _C(); // Not reserved
> > > };
> > > 
> > > unsigned operator "" huttah(unsigned long long); // Reserved 
> > > (http://eel.is/c++draft/usrlit.suffix#1)
> > > 
> > > unsigned operator "" _W(unsigned long long); // Reserved
> > > unsigned operator "" _w(unsigned long long); // Reserved
> > > 
> > > static unsigned operator "" _X(unsigned long long); // Not reserved
> > > static unsigned operator "" _x(unsigned long long); // Not reserved
> > > ```
> > I think some of these tests are still missing. I'm especially worried about 
> > the user-defined literal cases being diagnosed, as we'd be warning to not 
> > do the exact thing users are expected to do.
> User defined literal tested below and behave as expected.
@aaron.ballman, the "not reserved" comment re: `_X` for the literal operator 
using the `operator string-literal identifier` form above seems suspect to me. 
See `_Bq` example in [over.literal].


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

https://reviews.llvm.org/D93095

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

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


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

https://reviews.llvm.org/D93901

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D93377#2481312 , @rjmccall wrote:

> Are you committed to the name `__ibm128`?

Insofar as that's what GCC on Power (for example, `gcc (Ubuntu 
7.5.0-3ubuntu1~18.04) 7.5.0` from 2017) has shipped with for several years, yes.


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

https://reviews.llvm.org/D93377

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

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:65-67
+  // The #pragma align/pack affected a record in an included file,  so Clang
+  // should warn when that pragma was written in a file that included the
+  // included file.

Remove extra space; add missing "the".



Comment at: clang/lib/Sema/SemaAttr.cpp:365-367
+  // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+  // information, diagnostics below might not be accurate if we have mixed
+  // pragmas.

It may be helpful to pre-commit tests with the incorrect behaviour as part of 
the NFC patch.



Comment at: clang/lib/Sema/SemaAttr.cpp:385-387
+  // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+  // information, diagnostics below might not be accurate if we have mixed
+  // pragmas.

Same comment re: pre-committing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93901

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

[PATCH] D93999: [clang] Fix message text for `-Wpointer-sign` to account for plain char

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: aaron.ballman, rsmith.
Herald added subscribers: jfb, jvesely.
hubert.reinterpretcast requested review of this revision.
Herald added a project: clang.

The `-Wpointer-sign` warning text is inappropriate for describing the 
incompatible pointer conversion between plain `char` and explicitly 
`signed`/`unsigned` `char` (whichever plain `char` has the same range as) and 
vice versa.

Specifically, in part, it reads "converts between pointers to integer types 
with different sign". This patch changes that portion to read instead as 
"converts between pointers to integer types that differ by 
signed/unsigned/plain variation".

C17 subclause 6.5.16.1 indicates that the conversions resulting in 
`-Wpointer-sign` warnings in assignment-like contexts are constraint 
violations. This means that strict conformance requires a diagnostic for the 
case where the message text is wrong before this patch. The lack of an even 
more specialized warning group is consistent with GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93999

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Sema/incompatible-sign.c
  clang/test/Sema/incompatible-sign.cpp
  clang/test/Sema/vector-ops.c
  clang/test/SemaObjC/objc-cf-audited-warning.m
  clang/test/SemaOpenCL/builtins-amdgcn-error.cl

Index: clang/test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- clang/test/SemaOpenCL/builtins-amdgcn-error.cl
+++ clang/test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -156,7 +156,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_inc32(, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   int signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_inc32(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_inc32(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_inc64() {
@@ -170,7 +170,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_inc64(, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   __INT64_TYPE__ signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_inc64(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_inc64(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_dec32() {
@@ -184,7 +184,7 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_dec32(, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   int signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_dec32(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_dec32(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private int *' to parameter of type 'volatile __private unsigned int *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
 
 void test_atomic_dec64() {
@@ -198,5 +198,5 @@
   const char ptr[] = "workgroup";
   val = __builtin_amdgcn_atomic_dec64(, val, __ATOMIC_ACQUIRE, ptr); // expected-error {{expression is not a string literal}}
   __INT64_TYPE__ signedVal = 15;
-  signedVal = __builtin_amdgcn_atomic_dec64(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types with different sign}}
+  signedVal = __builtin_amdgcn_atomic_dec64(, signedVal, __ATOMIC_ACQUIRE, ""); // expected-warning {{passing '__private long *' to parameter of type 'volatile __private unsigned long *' converts between pointers to integer types that differ by signed/unsigned/plain variation}}
 }
Index: clang/test/SemaObjC/objc-cf-audited-warning.m
===
--- clang/test/SemaObjC/objc-cf-audited-warning.m
+++ clang/test/SemaObjC/objc-cf-audited-warning.m
@@ -20,5 +20,5 @@
 
 void saveImageToJPG(const char *filename)
 {
-CFURLRef 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

qiucf wrote:
> hubert.reinterpretcast wrote:
> > nemanjai wrote:
> > > hubert.reinterpretcast wrote:
> > > > I think this function should vocally fail when presented with 
> > > > "unordered" cases.
> > > But is it possible to emit errors here or should there be code explicitly 
> > > added to Sema to disallow conversions between `__ibm128` and `__float128` 
> > > (and `long double` to/from either of those to which it is not equivalent)?
> > I did not mean a user-facing error message. I meant that there should be 
> > some form of assertion or internal diagnostic here. I believe `assert` is 
> > appropriate.
> > 
> > I will note that this is a public member function of ASTContext. Regardless 
> > of explicit code in Sema that does what you describe, I think this function 
> > should not present an interface where it does not report "unordered" cases 
> > as unordered.
> > 
> Adding assertion here makes `unsupportedTypeConversion` always fail in such 
> cases.
Yes, I know. I think `unsupportedTypeConversion` should avoid calling this 
function when it is possibly dealing with an "unordered" case unless if this 
function has a way of indicating "unordered" as a result. If there is a helper 
function for testing the unordered case in this class, then I think 
`unsupportedTypeConversion` can simply say that all ordered cases are supported 
(before doing more to figure out the unordered cases that are safe).



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

qiucf wrote:
> hubert.reinterpretcast wrote:
> > Not sure what the best method is to implement this, but `long double` and 
> > `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in 
> > effect.
> Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I saw 
> `__float128` and `long double` are the same for GCC but not for clang.
Have you checked whether the new libstdc++ for which this support is being 
added needs the GCC behaviour to work properly?

The GCC behaviour allows the following to be compiled without introducing novel 
overload resolution tiebreakers:
```
void f(__float128);
void f(__ibm128);
void f(int);

long double ld;

int main() { f(ld); }
```



Comment at: clang/lib/Sema/SemaType.cpp:1562-1563
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << 
"__ibm128";

qiucf wrote:
> hubert.reinterpretcast wrote:
> > Do the SYCL and OpenMP device exceptions to the error really apply for 
> > `__ibm128`?
> If host supports `__ibm128` but device does not?
Can you add a test that makes use of this? Also, there should be a case that 
triggers `err_device_unsupported_type`.


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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

nemanjai wrote:
> hubert.reinterpretcast wrote:
> > I think this function should vocally fail when presented with "unordered" 
> > cases.
> But is it possible to emit errors here or should there be code explicitly 
> added to Sema to disallow conversions between `__ibm128` and `__float128` 
> (and `long double` to/from either of those to which it is not equivalent)?
I did not mean a user-facing error message. I meant that there should be some 
form of assertion or internal diagnostic here. I believe `assert` is 
appropriate.

I will note that this is a public member function of ASTContext. Regardless of 
explicit code in Sema that does what you describe, I think this function should 
not present an interface where it does not report "unordered" cases as 
unordered.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > There's a lot of missing testing, especially around implicit conversions, 
> > narrowing conversions, and the usual arithmetic conversions.
> > 
> Make this a `constexpr` function and call it from the initializer of a global 
> declared with `consteval`.
> Make this a `constexpr` function and call it from the initializer of a global 
> declared with `consteval`.

Sorry, I meant `constinit` and not `consteval`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

I think this function should vocally fail when presented with "unordered" cases.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:803
+  case BuiltinType::Ibm128:
 // FIXME: For targets where long double and __float128 have the same size,
 // they are currently indistinguishable in the debugger without some

Update the comment for `__ibm128`.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

Not sure what the best method is to implement this, but `long double` and 
`__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in effect.



Comment at: clang/lib/Sema/SemaExpr.cpp:1188-1190
 /// Diagnose attempts to convert between __float128 and long double if
 /// there is no support for such conversion. Helper function of
 /// UsualArithmeticConversions().

Comment needs update.



Comment at: clang/lib/Sema/SemaExpr.cpp:1229
+   ::APFloat::PPCDoubleDouble())) ||
+ EitherIbm128;
 }

Same comment about possible blocking of `double` to `__ibm128` conversions.



Comment at: clang/lib/Sema/SemaExpr.cpp:1546-1547
 
   // Diagnose attempts to convert between __float128 and long double where
   // such conversions currently can't be handled.
   if (unsupportedTypeConversion(*this, LHSType, RHSType))

Comment needs update.



Comment at: clang/lib/Sema/SemaOverload.cpp:1887
+  FromType == S.Context.Ibm128Ty || ToType == S.Context.Ibm128Ty;
+  if ((Float128AndLongDouble &&
+   ((S.Context.LongDoubleTy) ==

This seems to disable conversion from `double` to `__ibm128`?



Comment at: clang/lib/Sema/SemaType.cpp:1562-1563
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << 
"__ibm128";

Do the SYCL and OpenMP device exceptions to the error really apply for 
`__ibm128`?



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:6
+
+__ibm128 gf;
+static __ibm128 sgf;

What does the debug info look like?



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:24
+};
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {

Add a function that reads an `__ibm128` from varargs.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

There's a lot of missing testing, especially around implicit conversions, 
narrowing conversions, and the usual arithmetic conversions.




Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

hubert.reinterpretcast wrote:
> There's a lot of missing testing, especially around implicit conversions, 
> narrowing conversions, and the usual arithmetic conversions.
> 
Make this a `constexpr` function and call it from the initializer of a global 
declared with `consteval`.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:29
+
+template  struct T1 {
+  T mem1;

Add a case where `__ibm128` is used as the type of a non-type template 
parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

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

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast 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 \

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92278

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

[PATCH] D91455: [XCOFF][AIX] Generate LSDA data and compact unwind section on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM; thanks.




Comment at: llvm/test/CodeGen/PowerPC/aix-exception.ll:108-109
+; ASM: .byte   255 # @LPStart Encoding = 
omit
+; ASM32:   .byte   187 # @TType Encoding = 

+; ASM64:  .byte188 # @TType Encoding = 

+; ASM: .uleb128 L..ttbase0-L..ttbaseref0

jasonliu wrote:
> hubert.reinterpretcast wrote:
> > Is there a plan to update the annotation output here?
> updated to address comment. 
Thanks.


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

https://reviews.llvm.org/D91455

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

[PATCH] D91455: [XCOFF][AIX] Generate LSDA data and compact unwind section on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AIXException.cpp:35
+  //   unsigned long lsda; /* Pointer to LSDA */
+  //   unsigned long personality;  /* Pointerto the personality routine */
+  //   }

Typo: missing space.


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

https://reviews.llvm.org/D91455

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

[PATCH] D91455: [XCOFF][AIX] Generate LSDA data and compact unwind section on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/test/CodeGen/PowerPC/aix-exception.ll:108-109
+; ASM: .byte   255 # @LPStart Encoding = 
omit
+; ASM32:   .byte   187 # @TType Encoding = 

+; ASM64:  .byte188 # @TType Encoding = 

+; ASM: .uleb128 L..ttbase0-L..ttbaseref0

Is there a plan to update the annotation output here?


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

https://reviews.llvm.org/D91455

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

[PATCH] D91455: [XCOFF][AIX] Generate LSDA data and compact unwind section on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

> 2. AIX uses a new personality routine, named __xlcxx_personality_v1. It 
> doesn't use the GCC personality rountine, because the intractability is not 
> there yet on AIX.

@jasonliu, is "intractability" a typo/autocorrect problem?


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

https://reviews.llvm.org/D91455

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

[PATCH] D91596: [PowerPC] [Clang] Fix alignment of 128-bit floating types

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Reverting this patch makes va_arg with the default 128-bit (double-double) long 
double work again:

  #include 
  void abort(void);
  
  void foo(int x, ...) {
va_list ap;
va_start(ap, x);
long double ans = va_arg(ap, long double);
if (ans != 42.L) abort();
  }
  
  int main(void) {
foo(0, 42.L);
  }

This patch caused the above to abort.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91596

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

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:520
   Options.DataSections = CodeGenOpts.DataSections;
-  Options.IgnoreXCOFFVisibility = CodeGenOpts.IgnoreXCOFFVisibility;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;

DiggerLin wrote:
> sfertile wrote:
> > DiggerLin wrote:
> > > sfertile wrote:
> > > > DiggerLin wrote:
> > > > > jasonliu wrote:
> > > > > > DiggerLin wrote:
> > > > > > > DiggerLin wrote:
> > > > > > > > jasonliu wrote:
> > > > > > > > > DiggerLin wrote:
> > > > > > > > > > jasonliu wrote:
> > > > > > > > > > > DiggerLin wrote:
> > > > > > > > > > > > jasonliu wrote:
> > > > > > > > > > > > > Instead of just removing this line, should this get 
> > > > > > > > > > > > > replaced with the new LangOpts option?
> > > > > > > > > > > > I do not think we need a CodeGenOp of 
> > > > > > > > > > > > ignore-xcoff-visibility in clang, we only need the 
> > > > > > > > > > > > LangOpt of the ignore-xcoff-visilbity to control 
> > > > > > > > > > > > whether we will  generate the visibility in the IR,  
> > > > > > > > > > > > when the LangOpt of ignore-xcoff-visibility do not 
> > > > > > > > > > > > generate the visibility attribute of GV in the IR. it 
> > > > > > > > > > > > do not need CodeGenOp of ignore-xcoff-visibility any 
> > > > > > > > > > > > more for the clang .
> > > > > > > > > > > > 
> > > > > > > > > > > > we have still CodeGen ignore-xcoff-visibility op in  
> > > > > > > > > > > > llc.
> > > > > > > > > > > We removed the visibility from IR level with this patch. 
> > > > > > > > > > > But there is also visibility settings coming from CodeGen 
> > > > > > > > > > > part of clang, which needs to get ignore when we are 
> > > > > > > > > > > doing the code gen in llc. So I think you still need to 
> > > > > > > > > > > set the options correct for llc.
> > > > > > > > > > yes we have the set the options correct for llc in the code.
> > > > > > > > > > 
> > > > > > > > > > in the source file llvm/lib/CodeGen/CommandFlags.cpp, we 
> > > > > > > > > > have (in the patch https://reviews.llvm.org/D87451 add new 
> > > > > > > > > > option -mignore-xcoff-visibility) , the function
> > > > > > > > > > TargetOptions codegen::InitTargetOptionsFromCodeGenFlags() {
> > > > > > > > > > 
> > > > > > > > > > Options.IgnoreXCOFFVisibility = getIgnoreXCOFFVisibility(); 
> > > > > > > > > > ...}
> > > > > > > > > > 
> > > > > > > > > What I'm saying is... 
> > > > > > > > > I think we need a line like this:
> > > > > > > > > `Options.IgnoreXCOFFVisibility = 
> > > > > > > > > LangOpts.IgnoreXCOFFVisibility;`
> > > > > > > > > so that when you invoke clang, backend would get the correct 
> > > > > > > > > setting as well. 
> > > > > > > > I do not think so, from the clang FE, we do not generated the 
> > > > > > > > visibility in the IR. so there is no need these line.
> > > > > > > or we can say that because we do not set the hidden visibility 
> > > > > > > into the GlobalValue , so we do not need the 
> > > > > > > Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;
> > > > > > I think I mentioned this before, we could have extra visibility 
> > > > > > settings in clang/lib/CodeGen that's not depending on the existing 
> > > > > > visibility setting in the IR. (You could search for `setVisibility` 
> > > > > > there.) That was the reason we did it in llc first. 
> > > > > I will add Options.IgnoreXCOFFVisibility = 
> > > > > LangOpts.IgnoreXCOFFVisibility;  here.
> > > > > I think I mentioned this before, we could have extra visibility 
> > > > > settings in clang/lib/CodeGen that's not depending on the existing 
> > > > > visibility setting in the IR. (You could search for setVisibility 
> > > > > there.) That was the reason we did it in llc first.
> > > > 
> > > >  A lot of these are in places we wouldn't encounter with AIX, like for 
> > > > Objective-C code gen. But are others like [[ 
> > > > https://github.com/llvm/llvm-project/blob/b03ea054db1bcf9452b3a70e21d3372b6e58759a/clang/lib/CodeGen/ItaniumCXXABI.cpp#L2507
> > > >  | this]]  an issue? Should they be addressed in this patch?
> > > after I added the Options.IgnoreXCOFFVisibility = 
> > > LangOpts.IgnoreXCOFFVisibility , even there is  
> > > GV->setVisibility(llvm::GlobalValue::HiddenVisibility);  it do not effect 
> > > our output.
> > > 
> > > there is following code in the function void 
> > > PPCAIXAsmPrinter::emitLinkage(const GlobalValue *GV,
> > >MCSymbol *GVSym) const
> > > {
> > >  . 
> > >   if (!TM.getIgnoreXCOFFVisibility()) {
> > > switch (GV->getVisibility()) {
> > > 
> > > // TODO: "exported" and "internal" Visibility needs to go here.
> > > case GlobalValue::DefaultVisibility:
> > >   break;
> > > case GlobalValue::HiddenVisibility:
> > >   VisibilityAttr = MAI->getHiddenVisibilityAttr();
> > >   break;
> > > case 

[PATCH] D90187: [NFC] Remove max_align.c LIT testcase

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90187

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

[PATCH] D90063: [AIX] Also error on -G for link-only step

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!


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

https://reviews.llvm.org/D90063

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

[PATCH] D90063: [AIX] Also error on -G for link-only step

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Driver/aix-err-options.c:14-15
+
 // RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
 // RUN:   FileCheck --check-prefix=CHECK64 %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \

Replace duplicate compile-to-IR step with preprocess-only step.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90063

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

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:20
+// RUN: FileCheck -check-prefixes=COMMON-IR,VISIBILITY-IR %s
+
+int x = 66;

There's no testing for the interaction with `-mignore-xcoff-visibility`.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:29
+}
+
+// COMMON-ASM: mflr 0

The general issue is that the `nop` instruction is being omitted even when the 
visibility information is not generated into the object file. This is also a 
problem when the visibility is applied in the source code:
```
int x = 66;
#pragma GCC visibility push(hidden)
__attribute__((__noinline__)) inline void f() {
  x = 55;
}
#pragma GCC visibility pop
int bar() {
  f();
  return x;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89986

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

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!


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

https://reviews.llvm.org/D89910

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

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks!




Comment at: clang/test/Driver/aix-err-options.c:1
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck -check-prefix=CHECK32 %s

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Does this need a call to `not`?
> Using `not` will fail the testcase.
Got it; didn't know `-###` has such an effect on the return code.


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

https://reviews.llvm.org/D89897

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

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix_alloca_align.c:11
+void foo() {
+  char *ptr1 = (char *)alloca(sizeof(char) * 9);
+}

I'm not entirely sure, but can we try for size 32 and see if we get 16?


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

https://reviews.llvm.org/D89910

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

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4925
+  if (RawTriple.isOSAIX())
+if (Arg *A = Args.getLastArg(options::OPT_G)) {
+  D.Diag(diag::err_drv_unsupported_opt_for_target)

Either remove the braces here or add braces for the outer block.

Refer to the
```
// Use braces for the outer `if` since the nested `for` is braced.
```
example under 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89897

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

[PATCH] D89897: [AIX] Emit error for -G option on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Driver/aix-err-options.c:1
+// RUN: %clang -target powerpc32-ibm-aix-xcoff -### -S -emit-llvm -G 0 2>&1 %s 
| \
+// RUN:   FileCheck -check-prefix=CHECK32 %s

Does this need a call to `not`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89897

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

[PATCH] D89910: [AIX] Let alloca return 16 bytes alignment

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/aix_alloca_align.c:2
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck -check-prefix 32BIT %s
+

Minor nit: Use double-hyphen for long option and `=` instead of space between 
the option and the argument.



Comment at: clang/test/CodeGen/aix_alloca_align.c:5
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix-xcoff -S -emit-llvm < %s | \
+// RUN:   FileCheck -check-prefix 64BIT %s
+

Same comment.



Comment at: clang/test/CodeGen/aix_alloca_align.c:7
+
+typedef long unsigned int size_t;
+extern void *alloca(size_t __size) __attribute__((__nothrow__));

Minor comment: Use `__SIZE_TYPE__`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89910

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

[PATCH] D88659: [NFC] Fix the definition of SuitableAlign

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


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

https://reviews.llvm.org/D88659

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

[PATCH] D89684: [AIX] Add mvecnvol and mnovecnvol options to enable the AIX extended and default vector ABIs.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D89684#2341922 , 
@hubert.reinterpretcast wrote:

> `-mabi=aixvecextabi` was suggested when I discussed this with someone who is 
> familiar with GCC. The choice of `extabi` at the end is to match the spelling 
> of the `__EXTABI__` macro.

To reduce the length of the option, and with the hopes that future 
vector-related ABI extensions on Power have more specific naming than just 
being the "extended" ABI, the updated suggestion is `-mabi=vec-extabi`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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

[PATCH] D89612: NFC: Fix -Wsign-compare warnings on 32-bit builds

[Hubert Tong via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG134ffa8138c3: NFC: Fix -Wsign-compare warnings on 32-bit 
builds (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89612

Files:
  clang/lib/Serialization/ASTReaderStmt.cpp
  llvm/lib/CodeGen/StackMaps.cpp


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt * : E->children())
 Child = Record.readSubStmt();


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt * : E->children())
 Child = Record.readSubStmt();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89684: [AIX] Add mvecnvol and mnovecnvol options to enable the AIX extended and default vector ABIs.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

I have two concerns over the choice of the option form/name.

Firstly, there's an `-mabi=` option that is used to select for ABI extensions. 
The choice of using an `-mabi=` suboption for selecting between the extended 
vector ABI and the default vector ABI should be considered.
Secondly, the `vecnvol` name does not allude to its relation with the 
vector-extended ABI.

`-mabi=aixvecextabi` was suggested when I discussed this with someone who is 
familiar with GCC. The choice of `extabi` at the end is to match the spelling 
of the `__EXTABI__` macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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

[PATCH] D89684: [AIX] Add mvecnvol and mnovecnvol options to enable the AIX extended and default vector ABIs.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

There's frontend changes anyway, so there should be changes for the predefined 
macros?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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

[PATCH] D89443: [PowerPC][AIX] Make `__vector [un]signed long` an error

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rG126094485ab9: [PowerPC][AIX] Make `__vector [un]signed long` 
an error (authored by hubert.reinterpretcast).

Changed prior to commit:
  https://reviews.llvm.org/D89443?vs=298287=298878#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89443

Files:
  clang/lib/Sema/DeclSpec.cpp
  clang/test/Parser/altivec.c
  clang/test/Parser/cxx-altivec.cpp

Index: clang/test/Parser/cxx-altivec.cpp
===
--- clang/test/Parser/cxx-altivec.cpp
+++ clang/test/Parser/cxx-altivec.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple=powerpc-apple-darwin8 -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc-apple-darwin8 -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix -target-feature +altivec -fsyntax-only -verify=expected,aix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix -target-feature +altivec -fsyntax-only -verify=expected,aix -std=c++11 %s
 #include 
 
 __vector char vv_c;
@@ -55,19 +57,33 @@
 
 vector int v = (vector int)(-1);
 
+// These should have errors on AIX and warnings otherwise.
+__vector long vv_l; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector signed long vv_sl; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector unsigned long vv_ul;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector long int vv_li;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector signed long int vv_sli;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector unsigned long int vv_uli;  // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector long v_l;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector signed long v_sl;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector unsigned long v_ul;  // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector long int v_li;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector signed long int v_sli;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector unsigned long int v_uli; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+
 // These should have warnings.
-__vector long vv_l; // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector signed long vv_sl; // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector unsigned long vv_ul;   // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector long int vv_li;// expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector signed long int vv_sli;// expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector unsigned long int vv_uli;  // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-vector long v_l;// expected-warning {{Use of 'long' with '__vector' 

[PATCH] D89612: NFC: Fix -Wsign-compare warnings on 32-bit builds

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: dantrushin, hokein, daltenty, stevewan, 
jasonliu.
Herald added a subscriber: hiraditya.
Herald added projects: clang, LLVM.
hubert.reinterpretcast requested review of this revision.

Comparing 32-bit `ptrdiff_t` against 32-bit `unsigned` results in 
`-Wsign-compare` warnings for both GCC and Clang.

The warning for the cases in question appear to identify an issue where the 
`ptrdiff_t` value would be mutated via conversion to an unsigned type.

The warning is resolved by using the usual arithmetic conversions to safely 
preserve the value of the `unsigned` operand while trying to convert to a 
signed type. Host platforms where `unsigned` has the same width as `unsigned 
long long` will need to make a different change, but using an explicit cast has 
disadvantages that can be avoided for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89612

Files:
  clang/lib/Serialization/ASTReaderStmt.cpp
  llvm/lib/CodeGen/StackMaps.cpp


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt * : E->children())
 Child = Record.readSubStmt();


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt * : E->children())
 Child = Record.readSubStmt();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89443: [PowerPC][AIX] Make `__vector [un]signed long` an error

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: nemanjai, ZarkoCA, cebowleratibm.
Herald added a subscriber: shchenz.
Herald added a project: clang.
hubert.reinterpretcast requested review of this revision.

The semantics associated with `__vector [un]signed long` are neither 
consistently specified nor consistently implemented. The IBM XL compilers on 
AIX traditionally treated these as deprecated aliases for the corresponding 
`__vector int` type in both 32-bit and 64-bit modes. The newer, Clang-based, 
IBM XL compilers on AIX make usage of the previously deprecated types an error. 
This is also consistent with IBM XL C/C++ for Linux on Power (on little endian 
distributions).

In line with the above, this patch upgrades (on AIX) the deprecation of 
`__vector long` to become removal.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89443

Files:
  clang/lib/Sema/DeclSpec.cpp
  clang/test/Parser/altivec.c
  clang/test/Parser/cxx-altivec.cpp

Index: clang/test/Parser/cxx-altivec.cpp
===
--- clang/test/Parser/cxx-altivec.cpp
+++ clang/test/Parser/cxx-altivec.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple=powerpc-apple-darwin8 -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc-apple-darwin8 -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -target-feature +altivec -fsyntax-only -verify=expected,nonaix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc-ibm-aix -target-feature +altivec -fsyntax-only -verify=expected,aix -std=c++11 %s
+// RUN: %clang_cc1 -triple=powerpc64-ibm-aix -target-feature +altivec -fsyntax-only -verify=expected,aix -std=c++11 %s
 #include 
 
 __vector char vv_c;
@@ -55,19 +57,33 @@
 
 vector int v = (vector int)(-1);
 
+// These should have errors on AIX and warnings otherwise.
+__vector long vv_l; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector signed long vv_sl; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector unsigned long vv_ul;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector long int vv_li;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector signed long int vv_sli;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+__vector unsigned long int vv_uli;  // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector long v_l;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector signed long v_sl;// nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector unsigned long v_ul;  // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector long int v_li;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector signed long int v_sli;   // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+vector unsigned long int v_uli; // nonaix-warning {{Use of 'long' with '__vector' is deprecated}}
+// aix-error@-1 {{cannot use 'long' with '__vector'}}
+
 // These should have warnings.
-__vector long vv_l; // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector signed long vv_sl; // expected-warning {{Use of 'long' with '__vector' is deprecated}}
-__vector unsigned long vv_ul;   // expected-warning {{Use of 'long' with 

[PATCH] D89064: [AIX] Support two itanium alignment LIT testcases for AIX using regex

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with suggestion.




Comment at: clang/test/Layout/itanium-pack-and-align.cpp:19
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( 
preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]

If it works, use `?` in place of `*`. Do also for all of the other cases.


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

https://reviews.llvm.org/D89064

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

[PATCH] D88659: [FE]Split SuitableAlign into two parts

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D88659#2323631 , @jyknight wrote:

> ISTM that comment may be incorrect. At least, the precedent set by GCC 
> appears to be that BIGGEST_ALIGNMENT and alloca _both_ should follow vector 
> alignment requirements of the selected ISA even when that is greater than any 
> fundamental alignment requirement, and greater than malloc's alignment.

That would make sense. I guess this is really the alloca alignment value then? 
We can change the scope of this patch to simply correct the comment and add 
another patch to update the value on AIX to be 16. Do you believe the change to 
the "definition" of `SuitableAlign` needs a note to the mailing list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88659

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

[PATCH] D89064: [AIX] Disable two itanium alignment LIT testcases

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D89064#2322717 , @Xiangling_L wrote:

> In D89064#2320133 , 
> @hubert.reinterpretcast wrote:
>
>> Can we use a regex to make this also work in AIX?
>
> Sure we can also do that. May I ask is that because we prefer letting AIX 
> support as many LIT testcases as possible?

Yes. In particular, if there was a reason for these cases to get expanded in 
the future, we'd like AIX to be tested as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89064

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

[PATCH] D89064: [AIX] Disable two itanium alignment LIT testcases

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Can we use a regex to make this also work in AIX?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89064

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

[PATCH] D88659: [FE]Split SuitableAlign into two parts

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D88659#2318203 , @jyknight wrote:

> It seems like on AIX, `__BIGGEST_ALIGNMENT__` should just be set to 16, then. 
> I'm not sure why you want it to be 8?



  /// Return the alignment that is suitable for storing any
  /// object with a fundamental alignment requirement.

Vector types have extended (not fundamental) alignment on AIX, because `malloc` 
is not guaranteed to return addresses that are 16-byte aligned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88659

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

[PATCH] D88963: [SystemZ][z/OS] Add test of zero length bitfield type size larger than target zero length bitfield boundary

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88963

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

[PATCH] D88845: [SystemZ][z/OS] Set default alignment rules for z/OS target

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/zos-alignment.c:114
+union u2 {
+  long  :0;
+  short  a;

The testing for no-attribute potentially 8-byte aligning zero-width bitfields 
in a non-initial position is missing.

```
struct A {
   char x;
   long : 0;
   char q;
};
```

What does the above do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88845

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

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35ecc7fe49ba: [clang][Sema] Fix PR47676: Handle dependent 
AltiVec C-style cast (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/pr47676.cpp


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else
___
cfe-commits mailing list

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: pkubaj, nemanjai, jasonliu, 
aaron.ballman.
Herald added a project: clang.
hubert.reinterpretcast requested review of this revision.

Fix premature decision in the presence of type-dependent expression operands on 
whether AltiVec vector initializations from single expressions are "splat" 
operations.

Verify that the instantiation is able to determine the correct cast semantics 
for both the scalar type and the vector type case.

Note that, because the change only affects the single-expression case (and the 
target type is an AltiVec-style vector type), the replacement of a 
parenthesized list with a parenthesized expression does not change the 
semantics of the program in a program-observable manner.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88526

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/pr47676.cpp


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 

+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -7408,7 +7408,7 @@
 }
 if (PE || PLE->getNumExprs() == 1) {
   Expr *E = (PE ? PE->getSubExpr() : PLE->getExpr(0));
-  if (!E->getType()->isVectorType())
+  if (!E->isTypeDependent() && !E->getType()->isVectorType())
 isVectorLiteral = true;
 }
 else


Index: clang/test/SemaTemplate/pr47676.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/pr47676.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu \
+// RUN:-target-feature +altivec -fsyntax-only -ast-dump \
+// RUN:-xc++ < %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// Ensures that casts to AltiVec type with a dependent expression operand does
+// not hit the assertion failure reported in PR47676. Further checks that casts
+// to AltiVec type with a dependent expression operand is, on instantiation,
+// able to correctly differentiate between a splat case and a bitcast case.
+template  void f(T *tp) {
+  extern void g(int, ...);
+  g(0, (__vector int)(*tp));
+  g(0, (__vector int)*tp);
+}
+
+void g(void) {
+  f<__vector float>(nullptr);
+//  CHECK: | |-FunctionDecl {{.*}} f 'void (__vector float *)'
+
+//  CHECK: |   | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+//  CHECK: | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}}'__vector float' 
+
+  f(nullptr);
+//  CHECK: | `-FunctionDecl {{.*}} f 'void (double *)'
+
+//  CHECK: | | `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | |   `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: | | `-ImplicitCastExpr {{.*}}'double' 
+
+//  CHECK: |   `-CStyleCastExpr {{.*}} '__vector int' 
+// CHECK-NEXT: | `-ImplicitCastExpr {{.*}} 'int' 
+// CHECK-NEXT: |   `-ImplicitCastExpr {{.*}}:'double' 
+}
Index: 

[PATCH] D88500: [AIX][Clang][Driver] Link libm in c++ mode

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments.




Comment at: clang/test/Driver/aix-ld.c:406
 // CHECK-LD32-NOSTDLIBXX-LCXX: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOSTDLIBXX-LCXX-:"-lm"
 // CHECK-LD32-NOSTDLIBXX-LCXX: "-lc"

Typo.



Comment at: clang/test/Driver/aix-ld.c:446
 // CHECK-LD32-NOSTARTFILES-LCXX: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD32-NOSTARTFILES-LCXX  "-lc++"
+// CHECK-LD32-NOSTARTFILES-LCXX:  "-lc++"
 // CHECK-LD32-NOSTARTFILES-LCXX: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"

Formatting.



Comment at: clang/test/Driver/aix-ld.c:448
 // CHECK-LD32-NOSTARTFILES-LCXX: 
"[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-NOSTARTFILES-LCXX:  "-lm"
 // CHECK-LD32-NOSTARTFILES-LCXX: "-lc"

Same comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88500

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

[PATCH] D88500: [AIX][Clang][Driver] Link libm along with libc++

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:165
 
+// Since libc++ has dependencies on libm, if we have one then add the 
other.
+if (getToolChain().ShouldLinkCXXStdlib(Args))

Is that right?

My check of the libc++ from XL C/C++ for AIX 16.1.0 shows a dependency on 
`libC.a`, but not `libm.a`:
```
[183]   0xundef  IMP DS EXTref libC.a(shrcore.o) 
uncaught_exception__3stdFv
```

It seems `-lm` is just linked for C++ invocations. Other targets also pick up 
`-lm` for C++ invocations as a stdlib (not specifically a C++ stdlib).



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:168
+  CmdArgs.push_back("-lm");
 CmdArgs.push_back("-lc");
   }

Minor nit: Given the spacing in this block, I think a blank line would be 
appropriate to have before this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88500

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

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Off-list discussion seems to indicate that only the NFC portion of the patch 
was intended to be approved.
That is, the scope of this patch is supposed to be 
https://reviews.llvm.org/D88105?id=293625 plus formatting changes.


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

https://reviews.llvm.org/D88105

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

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Looks to me that this (since it is approved) completely replaces D88130 
. The patch description needs to be changed of 
course.
Not sure if the committer wants to try splitting out the non-AIX test as an NFC 
patch.


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

https://reviews.llvm.org/D88105

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

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88182

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

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Driver/aix-ld.c:215
 // CHECK-LD64-NO-DEFAULT-LIBS: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-NO-STD-LIB: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD64-NO-DEFAULT-LIBS: "-isysroot" "[[SYSROOT:[^"]+]]"

Copy/paste error?



Comment at: clang/test/Driver/aix-ld.c:237
 // CHECK-LD32-CXX-ARG-ORDER: {{.*}}clang{{.*}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-CXX-ARG-ORDERP: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD32-CXX-ARG-ORDER: "-isysroot" "[[SYSROOT:[^"]+]]"

Typo?



Comment at: clang/test/Driver/aix-ld.c:249
 // CHECK-LD32-CXX-ARG-ORDER: "-lc++"
+// CHECK-LD32-CXX-ARG-ORDERP:
"[[RESOURCE_DIR]]/lib/aix/libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-CXX-ARG-ORDER: "-lc"

Typo?



Comment at: clang/test/Driver/aix-rtlib.c:3
+// RUN: %clang -target powerpc-ibm-aix -print-libgcc-file-name 
-no-canonical-prefixes \
+// RUN:-resource-dir=%S/Inputs/resource_dir | FileCheck 
-check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix -print-libgcc-file-name 
-no-canonical-prefixes \

Remove "hard" tab. Indent to line up the hyphens.



Comment at: clang/test/Driver/aix-rtlib.c:5
+// RUN: %clang -target powerpc64-ibm-aix -print-libgcc-file-name 
-no-canonical-prefixes \
+// RUN:-resource-dir=%S/Inputs/resource_dir   | FileCheck 
-check-prefix=CHECK64 %s
+

Same. Plus remove extra spaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88182

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