[PATCH] D124039: [OpenMP] Add better testing for the linker wrapper

[Zibi Sarbino via Phabricator via cfe-commits]

zibi added inline comments.



Comment at: clang/test/Driver/linker-wrapper-image.c:8
+
+// OPENMP: @__start_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry
+// OPENMP-NEXT: @__stop_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry

On PowerPC I'm seeing the following error: pointing at `@` char:
clang/test/Driver/linker-wrapper-image.c:10:12: error: OPENMP: expected string 
not found in input
// OPENMP: @__start_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry

Are you aware of this?



Comment at: clang/test/Driver/linker-wrapper.c:9
+
+// NVPTX_LINK: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
+

Similar error to above, the coordinates are off by 2 because of subsequent 
changes:

clang/test/Driver/linker-wrapper.c:11:16: error: NVPTX_LINK: expected string 
not found in input
// NVPTX_LINK: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124039

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

[PATCH] D124039: [OpenMP] Add better testing for the linker wrapper

[Zibi Sarbino via Phabricator via cfe-commits]

zibi added inline comments.



Comment at: clang/test/Driver/linker-wrapper-image.c:8
+
+// OPENMP: @__start_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry
+// OPENMP-NEXT: @__stop_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry

jhuber6 wrote:
> jhuber6 wrote:
> > zibi wrote:
> > > On PowerPC I'm seeing the following error: pointing at `@` char:
> > > clang/test/Driver/linker-wrapper-image.c:10:12: error: OPENMP: expected 
> > > string not found in input
> > > // OPENMP: @__start_omp_offloading_entries = external hidden constant 
> > > %__tgt_offload_entry
> > > 
> > > Are you aware of this?
> > Do you have a link to a buildbot with the full error log? I'll try to test 
> > it locally by manually setting the triple.
> I tested this on a PPC64LE machine and couldn't reproduce, do you have any 
> more information?
Yes, those test cases do pass on external bots. At the time of reporting I did 
not know this affects only the internal builds. We will handle this internally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124039

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

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Zibi Sarbino via Phabricator via cfe-commits]

zibi added a comment.

LGTM, I just wonder if we can make an extra parameter to be default. I notice 
some places that is a default parameter but not in all instances. With default 
parameter some of the calls might be simplified if there is no need to override 
it.




Comment at: clang/lib/Frontend/FrontendActions.cpp:812
+  bool BinaryMode = false;
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  if (HostTriple.isOSWindows()) {

If we don't need Triple anywhere else except here we can remove the new for 
local variable and do crate it as part of the test.

```
if (HostTriple(LLVM_HOST_TRIPLE).isOSWindows()) {
...
```
or even

```
bool BinaryMode = (HostTriple(LLVM_HOST_TRIPLE).isOSWindows())  ? true : false;

```



Comment at: llvm/lib/Support/ToolOutputFile.cpp:50
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid

Can we avoid creating  `HostTriple` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Zibi Sarbino via Phabricator via cfe-commits]

zibi added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:771
+TempPath, fd, TempPath,
+llvm::sys::fs::all_read | llvm::sys::fs::all_write,
+Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);

The ` llvm::sys::fs::all_read | llvm::sys::fs::all_write` seems to be a default 
so if we make that parameter last we won't need to pass it and worry about the 
mode parameter which would be the second last default parameter.  Switch 
parameters only when you determine that indeed `tag` is more frequent parameter 
which need to be set comparing to `mode` parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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

[PATCH] D134034: [test] Use host platform specific error message substitution

[Zibi Sarbino via Phabricator via cfe-commits]

zibi accepted this revision.
zibi added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134034

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

[PATCH] D153580: [SystemZ][z/OS] Add support for z/OS link step (executable and shared libs)

[Zibi Sarbino via Phabricator via cfe-commits]

zibi accepted this revision.
zibi added a comment.

LGTM


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

https://reviews.llvm.org/D153580

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

[PATCH] D151828: Disable pr59765-modules-global-ctor-dtor.cppm on z/OS to make it unsupported.

[Zibi Sarbino via Phabricator via cfe-commits]

zibi created this revision.
Herald added a project: All.
zibi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[z/OS] Disable pr59765-modules-global-ctor-dtor.cppm


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151828

Files:
  clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm


Index: clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
===
--- clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
+++ clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
@@ -1,9 +1,9 @@
 // https://github.com/llvm/llvm-project/issues/59765
 // FIXME: Since the signature of the constructors/destructors is
 // different in different targets. The current CHECK can't work
-// well when targeting or running on AIX.
+// well when targeting or running on AIX and z/OS.
 // It would be better to add the corresponding test for other test.
-// UNSUPPORTED: system-aix
+// UNSUPPORTED: system-zos, system-aix
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t


Index: clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
===
--- clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
+++ clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
@@ -1,9 +1,9 @@
 // https://github.com/llvm/llvm-project/issues/59765
 // FIXME: Since the signature of the constructors/destructors is
 // different in different targets. The current CHECK can't work
-// well when targeting or running on AIX.
+// well when targeting or running on AIX and z/OS.
 // It would be better to add the corresponding test for other test.
-// UNSUPPORTED: system-aix
+// UNSUPPORTED: system-zos, system-aix
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151828: [z/OS] Disable pr59765-modules-global-ctor-dtor.cppm on z/OS to make it unsupported.

[Zibi Sarbino via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4f0f171d7f8: [z/OS] Disable 
pr59765-modules-global-ctor-dtor.cppm on z/OS to make it… (authored by zibi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151828

Files:
  clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm


Index: clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
===
--- clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
+++ clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
@@ -1,9 +1,9 @@
 // https://github.com/llvm/llvm-project/issues/59765
 // FIXME: Since the signature of the constructors/destructors is
 // different in different targets. The current CHECK can't work
-// well when targeting or running on AIX.
+// well when targeting or running on AIX and z/OS.
 // It would be better to add the corresponding test for other test.
-// UNSUPPORTED: system-aix
+// UNSUPPORTED: system-zos, system-aix
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t


Index: clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
===
--- clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
+++ clang/test/CodeGenCXX/pr59765-modules-global-ctor-dtor.cppm
@@ -1,9 +1,9 @@
 // https://github.com/llvm/llvm-project/issues/59765
 // FIXME: Since the signature of the constructors/destructors is
 // different in different targets. The current CHECK can't work
-// well when targeting or running on AIX.
+// well when targeting or running on AIX and z/OS.
 // It would be better to add the corresponding test for other test.
-// UNSUPPORTED: system-aix
+// UNSUPPORTED: system-zos, system-aix
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152016: Remove 3-byte characters causing clang-tblgen to get I/O error.

[Zibi Sarbino via Phabricator via cfe-commits]

zibi created this revision.
zibi added reviewers: Kai, fanbo-meng, abhina.sreeskantharajan.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
zibi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[SystemZ} This revision fixes the following error caused by 
301eb6b68f30074ee3a90e2dfbd11dfd87076323 
.
LLVM ERROR: IO failure on output stream: EDC5122I Input/output error.

The characters seems to be 3-byte characters which cause the failure with auto 
conversion from EBCDIC to ASCII.
Credit to @Kai who found this issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152016

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6559,7 +6559,7 @@
 
 The ``__arm_streaming`` keyword is only available on AArch64 targets.
 It applies to function types and specifies that the function has a
-“streaming interface”.  This means that:
+"streaming interface".  This means that:
 
 * the function requires the Scalable Matrix Extension (SME)
 
@@ -6578,7 +6578,7 @@
 that switches into streaming mode before calling the function and
 switches back to non-streaming mode on return.
 
-``__arm_streaming`` can appear anywhere that a standard ``[[…]]`` type
+``__arm_streaming`` can appear anywhere that a standard ``[[...]]`` type
 attribute can appear.
 
 See `Arm C Language Extensions `_


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6559,7 +6559,7 @@
 
 The ``__arm_streaming`` keyword is only available on AArch64 targets.
 It applies to function types and specifies that the function has a
-“streaming interface”.  This means that:
+"streaming interface".  This means that:
 
 * the function requires the Scalable Matrix Extension (SME)
 
@@ -6578,7 +6578,7 @@
 that switches into streaming mode before calling the function and
 switches back to non-streaming mode on return.
 
-``__arm_streaming`` can appear anywhere that a standard ``[[…]]`` type
+``__arm_streaming`` can appear anywhere that a standard ``[[...]]`` type
 attribute can appear.
 
 See `Arm C Language Extensions `_
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152016: Remove 3-byte characters causing clang-tblgen to get I/O error.

[Zibi Sarbino via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a27ddd42333: Remove 3-byte characters causing clang-tblgen 
to get I/O error. (authored by zibi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152016

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6559,7 +6559,7 @@
 
 The ``__arm_streaming`` keyword is only available on AArch64 targets.
 It applies to function types and specifies that the function has a
-“streaming interface”.  This means that:
+"streaming interface".  This means that:
 
 * the function requires the Scalable Matrix Extension (SME)
 
@@ -6578,7 +6578,7 @@
 that switches into streaming mode before calling the function and
 switches back to non-streaming mode on return.
 
-``__arm_streaming`` can appear anywhere that a standard ``[[…]]`` type
+``__arm_streaming`` can appear anywhere that a standard ``[[...]]`` type
 attribute can appear.
 
 See `Arm C Language Extensions `_


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -6559,7 +6559,7 @@
 
 The ``__arm_streaming`` keyword is only available on AArch64 targets.
 It applies to function types and specifies that the function has a
-“streaming interface”.  This means that:
+"streaming interface".  This means that:
 
 * the function requires the Scalable Matrix Extension (SME)
 
@@ -6578,7 +6578,7 @@
 that switches into streaming mode before calling the function and
 switches back to non-streaming mode on return.
 
-``__arm_streaming`` can appear anywhere that a standard ``[[…]]`` type
+``__arm_streaming`` can appear anywhere that a standard ``[[...]]`` type
 attribute can appear.
 
 See `Arm C Language Extensions `_
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

[Zibi Sarbino via Phabricator via cfe-commits]

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

LTGM, thx for an extra mile, Abhina.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

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