[PATCH] D46443: Add missing cstdalign header

2021-02-16 Thread 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

2021-02-02 Thread 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

2021-02-02 Thread 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

2021-02-02 Thread 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

2021-02-01 Thread 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

2021-02-01 Thread 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

2021-01-30 Thread 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.

2021-01-29 Thread 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.

2021-01-29 Thread 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

2021-01-23 Thread 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

2021-01-23 Thread 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

2021-01-22 Thread 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

2021-01-21 Thread 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

2021-01-11 Thread 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

2021-01-09 Thread 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

2021-01-08 Thread 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

2021-01-08 Thread 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

2021-01-08 Thread 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

2021-01-06 Thread 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

2021-01-06 Thread 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

2021-01-03 Thread 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

2021-01-03 Thread 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

2020-12-30 Thread 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

2020-12-22 Thread 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

2020-12-19 Thread 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

2020-12-18 Thread 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

2020-12-02 Thread 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

2020-12-01 Thread 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

2020-12-01 Thread 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

2020-12-01 Thread 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

2020-11-30 Thread 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

2020-11-28 Thread 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

2020-10-27 Thread 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

2020-10-26 Thread 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

2020-10-26 Thread 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

2020-10-23 Thread 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=*

2020-10-22 Thread 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

2020-10-22 Thread 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

2020-10-22 Thread 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

2020-10-22 Thread 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

2020-10-21 Thread 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

2020-10-21 Thread 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

2020-10-21 Thread 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

2020-10-21 Thread 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.

2020-10-20 Thread 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

2020-10-20 Thread 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.

2020-10-20 Thread 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.

2020-10-19 Thread 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

2020-10-18 Thread 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

2020-10-16 Thread 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

2020-10-14 Thread 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

2020-10-13 Thread 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

2020-10-10 Thread 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

2020-10-09 Thread 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

2020-10-08 Thread 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

2020-10-08 Thread 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

2020-10-07 Thread 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

2020-10-06 Thread 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

2020-10-01 Thread 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

2020-09-29 Thread 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

2020-09-29 Thread 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++

2020-09-29 Thread 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

2020-09-24 Thread 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

2020-09-24 Thread 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

2020-09-24 Thread 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

2020-09-23 Thread 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


[PATCH] D87927: [AIX][clang][driver] Make sure crti[_64].o is linked in C++ mode

2020-09-21 Thread 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/D87927/new/

https://reviews.llvm.org/D87927

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


[PATCH] D87927: [AIX][clang][driver] Make sure ctri.o is linked in C++ mode

2020-09-21 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Driver/aix-ld.c:16
 // CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000"
 // CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"

There should be checks for not having `crti*.o` on the C invocations (at least 
insofar as we decided that we were going to have checks for not having `-lc++`).



Comment at: clang/test/Driver/aix-ld.c:321
 // CHECK-LD32-NOSTDLIBXX-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-NOSTDLIBXX-LCXX: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD32-NOSTDLIBXX-LCXX: "-L[[SYSROOT]]/usr/lib"

I think this is right but is worth noting. The dynamic 
initialization/finalization functionality is of a more fundamental nature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87927

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


[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

2020-09-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87611

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


[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

2020-09-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7216
+  "aligned %select{allocation|deallocation}0 function of type '%1' is "
+  "%select{only|not}4 available on %2 %select{%3 or newer|}4">;
 def note_silence_aligned_allocation_unavailable : Note<

Can something be done about the trailing space?



Comment at: clang/test/SemaCXX/unavailable_aligned_allocation.cpp:158
+#elif defined(ZOS)
+// expected-error@-18 {{aligned deallocation function of type 'void (void *, 
enum std::align_val_t) noexcept' is not available on z/OS}}}
+// expected-note@-19 {{if you supply your own aligned allocation functions}}

Try to use the regex form to check against trailing whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87611

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


[PATCH] D87624: [SystemZ][z/OS] Set default wchar_t type for zOS

2020-09-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87624

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


[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

2020-09-14 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/ZOS.cpp:25-27
+void ZOS::addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const {

Please be consistent in using/not using additional namespace qualification. The 
declaration above this one takes advantage of the using directive for 
`llvm::opt`.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:1841
-<< IsDelete << FD.getType().getAsString() << OSName
-<< alignedAllocMinVersion(T.getOS()).getAsString();
 Diag(Loc, diag::note_silence_aligned_allocation_unavailable);

Just to ensure we are on the same page:
Passing `-Xclang -faligned-alloc-unavailable` on the non-Apple platforms does 
very bad things (hits "unreachable" and otherwise falls off the end of a 
function without initializing a return value). Part of this patch makes passing 
`-Xclang -faligned-alloc-unavailable` okay on z/OS.


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

https://reviews.llvm.org/D87611

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


[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

2020-09-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1006
  "vexpandbm $vD, $vB", IIC_VecGeneral,
- []>;
+ [(set v16i8:$vD, 
(int_ppc_altivec_vexpandbm
+  v16i8:$vB))]>;

The commit added "physical" tab characters instead of spaces here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82727

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


[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

2020-09-01 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

Confirming LGTM. @abhina.sreeskantharajan, it seems that you have a good number 
of commits to the project (I see at least three). If you do not yet have commit 
access, it may be appropriate to request it now so that you can push this 
change directly when you believe you have the feedback that you're looking for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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


[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

2020-08-31 Thread 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/lib/Driver/ToolChains/ZOS.cpp:15
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;

There should be no need for this using directive. The lookup following the 
//declarator-id// in the declarations should operate in the context of the 
namespace that "owns" the function. There's a chance it's needed because of 
MSVC build problems though.



Comment at: clang/lib/Driver/ToolChains/ZOS.cpp:18
+using namespace llvm::opt;
+using namespace clang;
+

Same comment.



Comment at: clang/lib/Driver/ToolChains/ZOS.h:25
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }

abhina.sreeskantharajan wrote:
> hubert.reinterpretcast wrote:
> > According to the RFC re: LLVM on z/OS, the initial support in LLVM for z/OS 
> > is only for XPLink. My understanding is that all XPLink applications are 
> > DLL-enabled. Does being DLL-enabled not imply that the code is 
> > position-independent?
> > 
> > I understand that the value of the `__DLL__` predefined macro from the XL C 
> > compiler does not reflect the implicit DLL-enablement of XPLink code; 
> > however, I also note that the same compiler claims falsely that `Option 
> > NODLL is ignored because option XPLINK is specified` when `-qnodll` does 
> > actually suppress the effect of an earlier `-qdll` in causing `__DLL__` to 
> > be defined.
> This is not always true because we do not require code to be PIC on z/OS, 
> even for XPLink applications. Absolute addresses may be present in code 
> sections for easier access (e.g. in calls to linkages, branch tables). We 
> also may link to libraries that contain non-PIC code.
Got it (I think). Load-time relocations can occur within the program text 
instead of using PIC code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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


[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

2020-08-27 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/ZOS.h:25
+
+  bool isPICDefault() const override { return false; }
+  bool isPIEDefault() const override { return false; }

According to the RFC re: LLVM on z/OS, the initial support in LLVM for z/OS is 
only for XPLink. My understanding is that all XPLink applications are 
DLL-enabled. Does being DLL-enabled not imply that the code is 
position-independent?

I understand that the value of the `__DLL__` predefined macro from the XL C 
compiler does not reflect the implicit DLL-enablement of XPLink code; however, 
I also note that the same compiler claims falsely that `Option NODLL is ignored 
because option XPLINK is specified` when `-qnodll` does actually suppress the 
effect of an earlier `-qdll` in causing `__DLL__` to be defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86707

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


[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-25 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97ccf93b3615: [SystemZ][z/OS] Add z/OS Target and define 
macros (authored by abhina.sreeskantharajan, committed by 
hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-zos.c


Index: clang/test/Preprocessor/init-zos.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding 
-triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-GNUXX:#define __DLL__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-GNUXX:#define __wchar_t 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -728,6 +728,55 @@
   bool defaultsToAIXPowerAlignment() const override { return true; }
 };
 
+// z/OS target
+template 
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions , const llvm::Triple ,
+MacroBuilder ) const override {
+// FIXME: _LONG_LONG should not be defined under -std=c89.
+Builder.defineMacro("_LONG_LONG");
+Builder.defineMacro("_OPEN_DEFAULT");
+// _UNIX03_WITHDRAWN is required to build libcxx.
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");
+Builder.defineMacro("__BFP__");
+// FIXME: __BOOL__ should not be defined under -std=c89.
+Builder.defineMacro("__BOOL__");
+Builder.defineMacro("__LONGNAME__");
+Builder.defineMacro("__MVS__");
+Builder.defineMacro("__THW_370__");
+Builder.defineMacro("__THW_BIG_ENDIAN__");
+Builder.defineMacro("__TOS_390__");
+Builder.defineMacro("__TOS_MVS__");
+Builder.defineMacro("__XPLINK__");
+
+if (this->PointerWidth == 64)
+  Builder.defineMacro("__64BIT__");
+
+if (Opts.CPlusPlus) {
+  Builder.defineMacro("__DLL__");
+  // _XOPEN_SOURCE=600 is required to build libcxx.
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}
+
+if (Opts.GNUMode) {
+  Builder.defineMacro("_MI_BUILTIN");
+  Builder.defineMacro("_EXT");
+}
+
+if (Opts.CPlusPlus && Opts.WChar) {
+  // Macro __wchar_t is defined so that the wchar_t data
+  // type is not declared as a typedef in system headers.
+  Builder.defineMacro("__wchar_t");
+}
+  }
+
+public:
+  ZOSTargetInfo(const llvm::Triple , const TargetOptions )
+  : OSTargetInfo(Triple, Opts) {}
+};
+
 void addWindowsDefines(const llvm::Triple , const LangOptions ,
MacroBuilder );
 
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -452,6 +452,8 @@
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::ZOS:
+  return new ZOSTargetInfo(Triple, Opts);
 default:
   return new SystemZTargetInfo(Triple, Opts);
 }


Index: clang/test/Preprocessor/init-zos.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// 

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-24 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D85324#2233290 , 
@abhina.sreeskantharajan wrote:

> Thanks Hubert, I fixed the comment.

Got it; I'll look into committing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-21 Thread 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 from my end; although @MaskRay might want another look.




Comment at: clang/lib/Basic/Targets/OSTargets.h:758
+  Builder.defineMacro("__DLL__");
+  // XOPEN_SOURCE=600 is required to build libcxx.
+  Builder.defineMacro("_XOPEN_SOURCE", "600");

Minor nit: s/XOPEN_SOURCE/_XOPEN_SOURCE/;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-20 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:743
+Builder.defineMacro("__BFP__");
+// FIXME: __BOOL__ should be defined under strict -std=c89.
+Builder.defineMacro("__BOOL__");

MaskRay wrote:
> What is strict -std=c89? `!Opts.C99` ?
The comment has a typo. The macro should //not// be defined with strict C89 
modes.

> What is strict -std=c89? `!Opts.C99` ?

In the context of this macro, "strict C89" means `!Opts.C99` and the severity 
of `ext_c99_feature` diagnostics is at least an error. This occurs, for 
example, with `-std=gnu89 -Werror=c99-extensions`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-20 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Preprocessor/init-zos.c:5
+//
+// S390X-ZOS-CXX:#define _EXT 1
+// S390X-ZOS-C99:#define _ISOC99_SOURCE 1

Should this be `GNUXX`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

2020-08-20 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:736
+MacroBuilder ) const override {
+// FIXME: LONG_LONG should not be defined under -std=c89
+Builder.defineMacro("_LONG_LONG");

Minor nit: Typo.

The FIXME is okay until there's a reason to fix this. The review necessary for 
addressing the FIXME deserves another patch anyway.

The situation between z/OS and AIX is different for this case. On AIX, the C 
ABI compatibility of `imaxdiv_t` is affected. In other words, on AIX, fixing 
this might cause surprising behaviour.



Comment at: clang/lib/Basic/Targets/OSTargets.h:743
+Builder.defineMacro("__BFP__");
+Builder.defineMacro("__BOOL__");
+Builder.defineMacro("__LONGNAME__");

Sorry for not catching this earlier, but this also needs a FIXME re: strict C89.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

2020-08-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D85938#2216905 , @lebedev.ri wrote:

> Thanks. Seems to fix the test for me FWIW, but
>
> 1. why wasn't this caught by anyone and all of the bots?

For what it's worth, I have been affected by this and was drafting a note to 
D82822  today to ask about the status.

> 2. what is the actual underlying problem, if any?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85938

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


[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

2020-08-13 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdedaf78fa714: [SystemZ][z/OS] enable trigraphs by default on 
z/OS (authored by abhina.sreeskantharajan, committed by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85722

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/trigraphs.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp


Index: clang/test/Lexer/cxx1z-trigraphs.cpp
===
--- clang/test/Lexer/cxx1z-trigraphs.cpp
+++ clang/test/Lexer/cxx1z-trigraphs.cpp
@@ -1,14 +1,31 @@
 // RUN: %clang_cc1 -std=c++1z %s -verify
-// RUN: %clang_cc1 -std=c++1z %s -ftrigraphs -fsyntax-only 2>&1 | FileCheck 
--check-prefix=TRIGRAPHS %s
+// RUN: %clang_cc1 -std=c++1z %s -verify -ftrigraphs -DENABLED_TRIGRAPHS=1
+// RUN: %clang_cc1 -std=c++1z %s -verify -fno-trigraphs -DENABLED_TRIGRAPHS=0
 
-??= define foo ; // expected-error {{}} expected-warning {{trigraph ignored}}
+#ifdef __MVS__
+#ifndef ENABLED_TRIGRAPHS
+#define ENABLED_TRIGRAPHS 1
+#endif
+#endif
 
-static_assert("??="[0] == '#', ""); // expected-error {{failed}} 
expected-warning {{trigraph ignored}}
+??= define foo ;
+
+static_assert("??="[0] == '#', "");
 
 // ??/
-error here; // expected-error {{}}
+error here;
 
-// Note, there is intentionally trailing whitespace two lines below.
-// TRIGRAPHS: :[[@LINE+1]]:{{.*}} backslash and newline separated by space
+// Note, there is intentionally trailing whitespace one line below.
 // ??/  
-error here; // expected-error {{}}
+error here;
+
+#if !ENABLED_TRIGRAPHS
+// expected-error@11 {{}} expected-warning@11 {{trigraph ignored}}
+// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}}
+// expected-error@16 {{}}
+// expected-error@20 {{}}
+#else
+// expected-warning@11 {{trigraph converted}}
+// expected-warning@13 {{trigraph converted}}
+// expected-warning@19 {{backslash and newline separated by space}}
+#endif
Index: clang/test/Frontend/trigraphs.cpp
===
--- clang/test/Frontend/trigraphs.cpp
+++ clang/test/Frontend/trigraphs.cpp
@@ -4,12 +4,14 @@
 // RUN: %clang_cc1 -DSTDCPP17 -std=c++1z -verify -fsyntax-only %s
 // RUN: %clang_cc1 -DSTDCPP17TRI -ftrigraphs -std=c++1z -verify -fsyntax-only 
%s
 // RUN: %clang_cc1 -DMSCOMPAT -fms-compatibility -std=c++11 -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -DNOTRI -fno-trigraphs -verify -fsyntax-only %s
 
 void foo() {
 #if defined(NOFLAGS) || defined(STDCPP11) || defined(STDGNU11TRI) || \
-defined(STDCPP17TRI)
+defined(STDCPP17TRI) || (defined(__MVS__) && !defined(NOTRI))
   const char c[] = "??/n"; // expected-warning{{trigraph converted to '\' 
character}}
-#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT)
+#elif defined(STDGNU11) || defined(STDCPP17) || defined(MSCOMPAT) || \
+defined(NOTRI)
   const char c[] = "??/n"; // expected-warning{{trigraph ignored}}
 #else
 #error Not handled.
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2787,7 +2787,9 @@
   // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs
   // is specified, or -std is set to a conforming mode.
   // Trigraphs are disabled by default in c++1z onwards.
-  Opts.Trigraphs = !Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17;
+  // For z/OS, trigraphs are enabled by default (without regard to the above).
+  Opts.Trigraphs =
+  (!Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17) || T.isOSzOS();
   Opts.Trigraphs =
   Args.hasFlag(OPT_ftrigraphs, OPT_fno_trigraphs, Opts.Trigraphs);
 


Index: clang/test/Lexer/cxx1z-trigraphs.cpp
===
--- clang/test/Lexer/cxx1z-trigraphs.cpp
+++ clang/test/Lexer/cxx1z-trigraphs.cpp
@@ -1,14 +1,31 @@
 // RUN: %clang_cc1 -std=c++1z %s -verify
-// RUN: %clang_cc1 -std=c++1z %s -ftrigraphs -fsyntax-only 2>&1 | FileCheck --check-prefix=TRIGRAPHS %s
+// RUN: %clang_cc1 -std=c++1z %s -verify -ftrigraphs -DENABLED_TRIGRAPHS=1
+// RUN: %clang_cc1 -std=c++1z %s -verify -fno-trigraphs -DENABLED_TRIGRAPHS=0
 
-??= define foo ; // expected-error {{}} expected-warning {{trigraph ignored}}
+#ifdef __MVS__
+#ifndef ENABLED_TRIGRAPHS
+#define ENABLED_TRIGRAPHS 1
+#endif
+#endif
 
-static_assert("??="[0] == '#', ""); // expected-error {{failed}} expected-warning {{trigraph ignored}}
+??= define foo ;
+
+static_assert("??="[0] == '#', "");
 
 // ??/
-error here; // expected-error {{}}
+error here;
 
-// Note, there is intentionally trailing whitespace two lines below.
-// TRIGRAPHS: :[[@LINE+1]]:{{.*}} backslash and newline separated by space
+// Note, there is intentionally trailing whitespace one 

[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

2020-08-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM with suggested changes.




Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:19
+// Note, there is intentionally trailing whitespace one lines below.
 // ??/
+error here;

Intentional whitespace disappeared.



Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24
+// expected-error@11 {{}} expected-warning@11 {{trigraph ignored}}
+// expected-error@13 {{}} {failed}} expected-warning@13 {{trigraph ignored}}
+// expected-error@16 {{}}

This seems to have changed in a weird way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85722

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


[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

2020-08-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24
+// notri-warning@6 {{trigraph converted}} tri-warning@6 {{trigraph converted}}
+// notri-warning@12 {{backslash and newline separated by space}} 
tri-warning@12 {{backslash and newline separated by space}}
+#endif

This strategy duplicates the trigraphs-enabled behaviour checks three times.

Plain `-verify` can be used along with a macro instead (without such 
duplication). This also allows the explicit `-fno-trigraphs` case to be added 
with little cost.

Pseudo-code:
```
// RUN: ...
// RUN: ... -ftrigraphs -DENABLED_TRIGRAPHS=1
// RUN: ... -fno-trigraphs -DENABLED_TRIGRAPHS=0

#ifdef __MVS__
#ifndef ENABLED_TRIGRAPHS
#define ENABLED_TRIGRAPHS 1
#endif
#endif

#if !ENABLED_TRIGRAPHS
// ...
#else
// ...
#endif
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85722

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


[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

2020-08-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2787-2789
   // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs
-  // is specified, or -std is set to a conforming mode.
+  // is specified, -std is set to a conforming mode, or on z/OS.
   // Trigraphs are disabled by default in c++1z onwards.

If going with an integrated update to the existing sentences, both of the 
original sentences would require an update (not just the first one). The first 
sentence is also meant to describe GCC's behavior, for which I am not sure 
there's a statement to be made for GCC on z/OS. I suggest to add an additional 
sentence instead.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2791
+  Opts.Trigraphs =
+  (!Opts.GNUMode && !Opts.MSVCCompat && !Opts.CPlusPlus17) || T.isOSzOS();
   Opts.Trigraphs =

I would like to point out that processing trigraphs when most platforms don't 
would be a portability concern. Clang appears to mitigate this somewhat with 
warnings.



Comment at: clang/test/Frontend/trigraphs.cpp:8
+// RUN: %clang_cc1 -DZOS -triple=s390x-none-zos -verify -fsyntax-only %s
+// RUN: %clang_cc1 -DZOSNOTRI -triple=s390x-none-zos -fno-trigraphs -verify 
-fsyntax-only %s
 

Do we know if `-fno-trigraphs` is meaningfully functional on z/OS? I believe 
trigraph usage might need to be replaced to use digraphs in the system headers 
before using `-fno-trigraphs` can be expected to work in a real user 
application.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85722

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


[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

2020-08-07 Thread 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 edits.




Comment at: clang/lib/Driver/ToolChains/AIX.cpp:213
+
+  llvm_unreachable("unexpected c++ library type, only libc++ is supported.");
+}

Minor nit: Follow normal capitalization rules when not formatting for a 
user-facing error message that falls under 
https://llvm.org/docs/CodingStandards.html#id15.



Comment at: clang/test/Driver/aix-ld.c:336-338
+// RUN:  -target powerpc-ibm-aix7.1.0.0 \
+// RUN:  -stdlib=libstdc++ \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \

Minor nit: We align the options to a utility on later lines with the options to 
the utility on earlier lines.



Comment at: clang/test/Driver/aix-ld.c:343-345
+// RUN:  -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:  -stdlib=libstdc++ \
+// RUN:  --sysroot %S/Inputs/aix_ppc_tree \




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

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


[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

2020-08-07 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

I confirm that I have reviewed the Clang part of this change, which is entirely 
consistent with the status quo for XCOFF. I'm not seeing any outstanding 
comments, and I do not believe that the changes are controversial. I think this 
patch meets the "likely community consensus" requirements for being committed.


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

https://reviews.llvm.org/D82081

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


[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

2020-08-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:210
+  case ToolChain::CST_Libstdcxx:
+llvm::report_fatal_error("linking libstdc++ unimplemented on AIX");
+  }

Should there be a test for the error message?



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:211
+llvm::report_fatal_error("linking libstdc++ unimplemented on AIX");
+  }
+}

I suggest using `return` inside the switch and having `llvm_unreachable` after 
the `switch` if the `switch` is expected to handle all of the possible cases.



Comment at: clang/test/Driver/aix-ld.c:18
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32: "-lc"
 

Should there be tests that the C++ library is not included when linking with a 
C invocation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

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


[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:732
+Builder.defineMacro("_OPEN_DEFAULT");
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");

This is not defined by z/OS XL C/C++. It seems that this is more of a macro to 
be defined by a user application (perhaps as part of its configuration/port for 
z/OS) and less a macro that should be predefined by the compiler.



Comment at: clang/lib/Basic/Targets/OSTargets.h:750
+
+if (Opts.C11 || Opts.GNUMode)
+  Builder.defineMacro("__IBM_UTF_LITERAL");

Shouldn't UTF literals be reported as being enabled under strict C++11?



Comment at: clang/lib/Basic/Targets/OSTargets.h:753
+
+if (Opts.C11 || (Opts.GNUMode && !Opts.CPlusPlus))
+  Builder.defineMacro("__IBMC_GENERIC");

Is this consistent with `__has_extension` with respect to `-pedantic-errors`?
That is, `-pedantic-errors` causes `__has_extension` to report the value that 
`__has_feature` would report. Compiler Explorer link: 
https://godbolt.org/z/EEn8rr

Same question for all of the other IBM-style feature test macros.



Comment at: clang/lib/Basic/Targets/OSTargets.h:758
+  Builder.defineMacro("__DLL__");
+  // Macro __wchar_t exposes the definition of wchar_t data type
+  // in system headers.

Should the comment instead say that `__wchar_t` should be defined so that the 
system headers do not try to declare `wchar_t` as a typedef?



Comment at: clang/lib/Basic/Targets/OSTargets.h:761
+  Builder.defineMacro("__wchar_t");
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}

Same comment as before re: macros that should be declared by the application.



Comment at: clang/lib/Basic/Targets/OSTargets.h:764
+
+if (Opts.C11 || Opts.CPlusPlus11 || Opts.GNUMode) 
+  Builder.defineMacro("__IBMC_NORETURN");

I don't see the relation between C++11 and `_Noreturn`. It's an extension in 
C++ that's available under, e.g., `-std=c++03`.



Comment at: clang/lib/Basic/Targets/OSTargets.h:767
+
+if (Opts.C11 || (Opts.GNUMode && Opts.CPlusPlus)) {
+  Builder.defineMacro("__IBM_CHAR16_T__");

`char16_t` is not a keyword in C11, so `__IBM_CHAR16_T__` should not be defined 
for C. Same re: `char32_t`.

Also, `char16_t` and `char32_t` are indeed keywords in C++11.



Comment at: clang/lib/Basic/Targets/OSTargets.h:770
+  Builder.defineMacro("__IBM_CHAR32_T__");
+  Builder.defineMacro("__IBMCPP_UTF_LITERAL__");
+}

The "IBMCPP" macro should not be defined in C modes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85315: [AIX][Clang] Add C++ linker option to the driver toolchain

2020-08-05 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:152
 
+  if (getToolChain().ShouldLinkCXXStdlib(Args))
+getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);

This is commonly added before `-lc`, etc. To be consistent with other targets, 
please switch the order of this block with the previous one.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:210
+  case ToolChain::CST_Libstdcxx:
+llvm_unreachable("linking libstdc++ unimplemented.");
+  }

Should this be a `report_fatal_error`? Note that the error message should not 
have a period at the end in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

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


[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-20 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

I've done another pass over the code (but did not get through the tests). I 
have no comments about the code at this time. My understanding is that 
@jasonliu will be doing another pass over this patch, so he can approve while 
I'm away on vacation.


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

https://reviews.llvm.org/D79719



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


[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

2020-07-17 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM; thanks.


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

https://reviews.llvm.org/D83974



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


[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

2020-07-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1212
   if (CodeGenOpts.RegisterGlobalDtorsWithAtExit) {
+if (getContext().getTargetInfo().getTriple().isOSAIX())
+  llvm::report_fatal_error(

This should query if the platform uses sinit/sterm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83974



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


[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

2020-07-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

Should we indicate planned removal in the Release Notes for version 11 and 
actual removal in those for version 12?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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


[PATCH] D83702: [AIX]Generate debug info for static init related functions

2020-07-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:248-250
   CGF.StartFunction(GlobalDecl(, DynamicInitKind::AtExit),
-CGM.getContext().VoidTy, fn, FI, FunctionArgList());
+CGM.getContext().VoidTy, fn, FI, FunctionArgList(),
+VD.getLocation(), VD.getInit()->getExprLoc());

dblaikie wrote:
> Any ideas why this is only showing up for AIX & not other targets, given this 
> code looks to be generic, not AIX-specific?
Yes, because the AIX implementation generates a direct call to the stub 
function on the finalization path when it finds that a stub was still 
registered with `atexit`.

That would explain why other platforms to not encounter this message:
```
inlinable function call in a function with debug info must have a !dbg location
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83702



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


[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1244
+if (!Base->Class->isEmpty() && !HandledFirstNonOverlappingEmptyField) {
+  IsFirstNonEmptyBase = true;
+  // By handling a base class that is not empty, we're handling the

`IsFirstNonEmptyBase` can be removed. It is set, but not used.


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

https://reviews.llvm.org/D79719



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


[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-09 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1246
+
+// AIX `power` alignment does not apply the preferred alignment for
+// non-union classes if the source of the alignment (the current base in

Move the comment to above the previous `if` and make the following `if` the 
`else` of the previous `if`.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1796
+  bool FoundFirstNonOverlappingEmptyFieldToHandle =
+  DefaultsToAIXPowerAlignment && FieldOffset == CharUnits::Zero() &&
+  !HandledFirstNonOverlappingEmptyField && !IsOverlappingEmptyField;

Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > Xiangling_L wrote:
> > > hubert.reinterpretcast wrote:
> > > > The condition is still more complex than I think it should be.
> > > > 
> > > > If we have found a "first" other-than-overlapping-empty-field, then we 
> > > > should set `HandledFirstNonOverlappingEmptyField` to `true` for 
> > > > non-union cases.
> > > > 
> > > > If `HandledFirstNonOverlappingEmptyField` being `false` is not enough 
> > > > for `FieldOffset == CharUnits::Zero()` to be true, then I think the 
> > > > correction would be to set `HandledFirstNonOverlappingEmptyField` in 
> > > > more places.
> > > > 
> > > > I would like to remove the check on `FieldOffset == CharUnits::Zero()` 
> > > > from here and instead have an assertion that 
> > > > `!HandledFirstNonOverlappingEmptyField` implies `FieldOffset == 
> > > > CharUnits::Zero()`.
> > > > 
> > > > Also, since we're managing `HandledFirstNonOverlappingEmptyField` in 
> > > > non-AIX cases, we should remove the `DefaultsToAIXPowerAlignment` 
> > > > condition for what is currently named 
> > > > `FoundFirstNonOverlappingEmptyFieldToHandle` (adjusting uses of it as 
> > > > necessary) and rename `FoundFirstNonOverlappingEmptyFieldToHandle` to 
> > > > `FoundFirstNonOverlappingEmptyField`.
> > > > Also, since we're managing HandledFirstNonOverlappingEmptyField in 
> > > > non-AIX cases, we should remove the DefaultsToAIXPowerAlignment 
> > > > condition for what is currently named 
> > > > FoundFirstNonOverlappingEmptyFieldToHandle 
> > > 
> > > I am not sure if we want to remove the `DefaultsToAIXPowerAlignment` 
> > > condition and bother with maintaining correct status of 
> > > `HandledFirstNonOverlappingEmptyField` for other targets.
> > > 
> > > We are actually claiming `HandledFirstNonOverlappingEmptyField` is an 
> > > auxiliary flag used for AIX only in its definition comments.
> > > 
> > > Besides, if we do want to manage `HandledFirstNonOverlappingEmptyField` 
> > > in non-AIX cases, I noticed that we have to set this flag to `true` 
> > > somewhere for objective-C++ cases. 
> > Okay, the other option I'm open is setting 
> > `HandledFirstNonOverlappingEmptyField` to `true` up front when not dealing 
> > with AIX `power` alignment.
> Thanks, that works too. I will address it in the next commit.
I'm not seeing the change for 
https://reviews.llvm.org/D79719?id=276143#inline-767942?



Comment at: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp:19
+  char x alignas(4)[8];
+} ;
+

Minor nit: Remove the space before the semicolon.



Comment at: clang/test/Layout/aix-Wpacked-no-diagnostics.cpp:15
+
+int a = sizeof(QQ);

Is there a reason to drop the `FileCheck` checking for the layout?


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

https://reviews.llvm.org/D79719



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


[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-08 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1208
+// "first (inherited) member".
+HandledFirstNonOverlappingEmptyField = true;
+

We need some sort of `IsFirstNonEmptyBase` to record that the current base 
qualifies for the alignment upgrade:
```
struct A { double x; };
struct B : A {} b;
```



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1245
+  // space or zero-extent array.
+  if (DefaultsToAIXPowerAlignment && HandledFirstNonOverlappingEmptyField) {
+UnpackedPreferredBaseAlign = UnpackedBaseAlign;

Query `!IsFirstNonEmptyBase` instead of `HandledFirstNonOverlappingEmptyField` 
here.


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

https://reviews.llvm.org/D79719



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


  1   2   3   4   5   >