[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 461598.
DiggerLin added a comment.

address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

Files:
  clang/test/lit.cfg.py


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -283,3 +283,13 @@
 config.environment['AIXTHREAD_STK'] = os.environ['AIXTHREAD_STK']
 elif platform.system() == 'AIX':
 config.environment['AIXTHREAD_STK'] = '4194304'
+
+# llvm-nm tools support an environment variable "OBJECT_MODE" on AIX OS, which
+# controls the kind of objects they will support. If there is no "OBJECT_MODE"
+# environment variable specified, the default behaviour is to support 32-bit
+# objects only. In order to not affect most test cases, which expect to support
+# 32-bit and 64-bit objects by default, set the environment variable
+# "OBJECT_MODE" to 'any' for llvm-nm on AIX OS.
+
+if 'system-aix' in config.available_features:
+config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -283,3 +283,13 @@
 config.environment['AIXTHREAD_STK'] = os.environ['AIXTHREAD_STK']
 elif platform.system() == 'AIX':
 config.environment['AIXTHREAD_STK'] = '4194304'
+
+# llvm-nm tools support an environment variable "OBJECT_MODE" on AIX OS, which
+# controls the kind of objects they will support. If there is no "OBJECT_MODE"
+# environment variable specified, the default behaviour is to support 32-bit
+# objects only. In order to not affect most test cases, which expect to support
+# 32-bit and 64-bit objects by default, set the environment variable
+# "OBJECT_MODE" to 'any' for llvm-nm on AIX OS.
+
+if 'system-aix' in config.available_features:
+config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 461589.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

Files:
  clang/test/lit.cfg.py


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -283,3 +283,6 @@
 config.environment['AIXTHREAD_STK'] = os.environ['AIXTHREAD_STK']
 elif platform.system() == 'AIX':
 config.environment['AIXTHREAD_STK'] = '4194304'
+
+if 'system-aix' in config.available_features:
+config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -283,3 +283,6 @@
 config.environment['AIXTHREAD_STK'] = os.environ['AIXTHREAD_STK']
 elif platform.system() == 'AIX':
 config.environment['AIXTHREAD_STK'] = '4194304'
+
+if 'system-aix' in config.available_features:
+config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Instead of applying `OBJECT_MODE` to everything, maybe we can apply it only to 
the tool invocations via a LIT expansion?
Something like `%llvm-nm` that expands (for AIX) to `OBJECT_MODE=any llvm-nm`?

Would need to check that the LIT internal shell accepts that syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

> It looks to me like 
> https://github.com/llvm/llvm-project/blob/b982ba2a6e0f11340b4e75d1d4eba9ff62a81df7/clang/lib/Driver/Driver.cpp#L554
>  should be modified to accept the OBJECT_MODE values you've implemented for 
> llvm-nm and llvm-ar. Otherwise, you'll never be able to use the any and 32_64 
> values supported by those tools as full environment variables (as opposed to 
> variables set for a single or limited set of commands) on a system where 
> clang is also used.

Those settings don't really make sense for clang since you need to pick a 
single target and aren't typically supported for a compiler driver on AIX. For 
example the XL compiler would give you:

  error: 1501-255 OBJECT_MODE setting is not recognized and is not a valid 
setting for the compiler.

if you try `OBJECT_MODE=any`, so I don't think most folks are using such an 
environment setting on AIX currently (i.e. typically they select either 32 or 
64).

I prefect the solution of adding something to the lit config rather than 
modifying individual tests, since I doubt most LLVM developers are not going to 
know to add -X any to their tests just for AIX. Ideally we’d find a way to get 
those tools to act in the AIX fashion, without placing the burden on developers 
to add options in their tests.

Maybe we can distinguish OBJECT_MODE settings for LLVM tools (which generally 
accept `any`) from a setting for the driver (which can't)? Something like a 
`TOOL_OBJECT_MODE` env which is preferred for the tools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin added a comment.

In D134284#3802806 , @daltenty wrote:

>> If OBJECT_MODE isn't an environment variable that clang uses, surely it 
>> should ignore it?
>
> The clang driver does in fact respect OBJECT_MODE on AIX, but I'm guessing it 
> doesn't accept the `any` setting (since it doesn't really make sense in that 
> context).
>
> Maybe it's better to leave the `llvm-nm` default as `any` even on AIX? It 
> doesn't match the system 'nm' but other wise we diverge from the `llvm-nm` 
> behaviour on other platforms it seems, and cause these test issues.

if change to default is Xany, the default value will be different with llvm-ar 
-X option(default value is -X32), that means llvm tool has different default -X 
option value on AIX OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[David Tenty via Phabricator via cfe-commits]

daltenty added a comment.

> If OBJECT_MODE isn't an environment variable that clang uses, surely it 
> should ignore it?

The clang driver does in fact respect OBJECT_MODE on AIX, but I'm guessing it 
doesn't accept the `any` setting (since it doesn't really make sense in that 
context).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[James Henderson via Phabricator via cfe-commits]

jhenderson added a subscriber: jasonliu.
jhenderson added a comment.

In D134284#3802793 , @DiggerLin wrote:

> In D134284#3802791 , @jhenderson 
> wrote:
>
>> In D134284#3802766 , @DiggerLin 
>> wrote:
>>
>>> In D134284#3802763 , @jhenderson 
>>> wrote:
>>>
 Wouldn't it be better to change the lit config to specify the 
 `OBJECT_MODE` environment variable on AIX?
>>>
>>> I have tried it before, I added the following in clang/test/lit.cfg.py
>>>
>>>   if 'system-aix' in config.available_features:
>>>config.environment['OBJECT_MODE'] = 'any' 
>>>
>>> it will cause  clang some problem in some test cases. something like 
>>> ,"clang-16: error: OBJECT_MODE setting any is not recognized and is not a 
>>> valid setting"
>>
>> Not sure I entirely follow. If `OBJECT_MODE` isn't an environment variable 
>> that clang uses, surely it should ignore it? What clang tests throw this 
>> sort of error?
>
> for example: clang/test/Parser/parser_overflow.c
>
>   Input was:
> 1: clang-16: error: OBJECT_MODE setting any is not recognized and is 
> not a valid setting

It looks to me like 
https://github.com/llvm/llvm-project/blob/b982ba2a6e0f11340b4e75d1d4eba9ff62a81df7/clang/lib/Driver/Driver.cpp#L554
 should be modified to accept the OBJECT_MODE values you've implemented for 
llvm-nm and llvm-ar. Otherwise, you'll never be able to use the `any` and 
`32_64` values supported by those tools as full environment variables (as 
opposed to variables set for a single or limited set of commands) on a system 
where clang is also used.

I'm neither a clang nor an AIX developer though, so my opinion is based only on 
what I've been reviewing in the llvm tools so far. Pinging @daltenty who 
implemented the functionality (see D82476 ) 
and @hubert.reinterpretcast and @jasonliu who reviewed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin added a comment.

In D134284#3802791 , @jhenderson 
wrote:

> In D134284#3802766 , @DiggerLin 
> wrote:
>
>> In D134284#3802763 , @jhenderson 
>> wrote:
>>
>>> Wouldn't it be better to change the lit config to specify the `OBJECT_MODE` 
>>> environment variable on AIX?
>>
>> I have tried it before, I added the following in clang/test/lit.cfg.py
>>
>>   if 'system-aix' in config.available_features:
>>config.environment['OBJECT_MODE'] = 'any' 
>>
>> it will cause  clang some problem in some test cases. something like 
>> ,"clang-16: error: OBJECT_MODE setting any is not recognized and is not a 
>> valid setting"
>
> Not sure I entirely follow. If `OBJECT_MODE` isn't an environment variable 
> that clang uses, surely it should ignore it? What clang tests throw this sort 
> of error?

for example: clang/test/Parser/parser_overflow.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

In D134284#3802766 , @DiggerLin wrote:

> In D134284#3802763 , @jhenderson 
> wrote:
>
>> Wouldn't it be better to change the lit config to specify the `OBJECT_MODE` 
>> environment variable on AIX?
>
> I have tried it before, I added the following in clang/test/lit.cfg.py
>
>   if 'system-aix' in config.available_features:
>config.environment['OBJECT_MODE'] = 'any' 
>
> it will cause  clang some problem in some test cases. something like 
> ,"clang-16: error: OBJECT_MODE setting any is not recognized and is not a 
> valid setting"

Not sure I entirely follow. If `OBJECT_MODE` isn't an environment variable that 
clang uses, surely it should ignore it? What clang tests throw this sort of 
error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin added a comment.

In D134284#3802763 , @jhenderson 
wrote:

> Wouldn't it be better to change the lit config to specify the `OBJECT_MODE` 
> environment variable on AIX?

I have tried it before, I added the following in clang/test/lit.cfg.py
if 'system-aix' in config.available_features:

  config.environment['OBJECT_MODE'] = 'any' 

it will cause  clang some problem in test cases. something like , clang do not 
know environment variable " OBJECT_MODE"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Wouldn't it be better to change the lit config to specify the `OBJECT_MODE` 
environment variable on AIX?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134284

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

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

[Digger Lin via Phabricator via cfe-commits]

DiggerLin created this revision.
DiggerLin added reviewers: jhenderson, daltenty, hubert.reinterpretcast.
Herald added subscribers: ormris, steven_wu, hiraditya.
Herald added a project: All.
DiggerLin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

since default object mode of llvm-nm is change to -X32 (from default  -Xany) in 
patch https://reviews.llvm.org/D132494. the test cases using the llvm-nm also 
need to change based on the new feature on AIX OS.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134284

Files:
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/CodeGen/thinlto-multi-module.ll
  clang/test/CodeGen/thinlto_backend.ll
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/driver-test3.c
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- clang/test/InterfaceStubs/weak.cpp
+++ clang/test/InterfaceStubs/weak.cpp
@@ -3,7 +3,7 @@
 // RUN: -interface-stub-version=ifs-v1 %s | \
 // RUN: FileCheck %s
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm -Xany - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
 // CHECK: Symbols:
Index: clang/test/InterfaceStubs/template-namespace-function.cpp
===
--- clang/test/InterfaceStubs/template-namespace-function.cpp
+++ clang/test/InterfaceStubs/template-namespace-function.cpp
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -o - -emit-interface-stubs %s | \
 // RUN: FileCheck %s
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm -Xany - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
 // CHECK: Symbols:
Index: clang/test/InterfaceStubs/inline.c
===
--- clang/test/InterfaceStubs/inline.c
+++ clang/test/InterfaceStubs/inline.c
@@ -3,33 +3,33 @@
 // RUN: -emit-interface-stubs -std=gnu89 -xc %s | \
 // RUN: FileCheck -check-prefix=CHECK-GNU %s
 // RUN: %clang -DINLINE=inline -target x86_64-linux-gnu -O0 -o - -c \
-// RUN: -std=gnu89 -xc %s | llvm-nm - | FileCheck -check-prefix=CHECK-GNU %s
+// RUN: -std=gnu89 -xc %s | llvm-nm -Xany - | FileCheck -check-prefix=CHECK-GNU %s
 
 // RUN: %clang_cc1 -DINLINE="__attribute__((always_inline))" \
 // RUN: -triple x86_64-unknown-linux-gnu -o - -emit-interface-stubs -xc %s | \
 // RUN: FileCheck -check-prefix=CHECK-GNU %s
 // RUN: %clang -DINLINE="__attribute__((always_inline))" \
 // RUN: -target x86_64-linux-gnu -O0 -o - -c -xc %s | \
-// RUN: llvm-nm - | FileCheck -check-prefix=CHECK-GNU %s
+// RUN: llvm-nm -Xany - | FileCheck -check-prefix=CHECK-GNU %s
 
 // RUN: %clang_cc1 -DINLINE=inline -triple x86_64-unknown-linux-gnu -o - \
 // RUN: -emit-interface-stubs -std=c99 -xc %s | \
 // RUN: FileCheck -check-prefix=CHECK-STD %s
 // RUN: %clang -DINLINE=inline -target x86_64-linux-gnu -O0 -o - -c -std=c99 \
-// RUN: -xc %s | llvm-nm - 2>&1 | count 0
+// RUN: -xc %s | llvm-nm -Xany - 2>&1 | count 0
 
 // RUN: %clang_cc1 -DINLINE="__attribute__((noinline))" \
 // RUN: -triple x86_64-unknown-linux-gnu -o - -emit-interface-stubs -std=c99 -xc %s | \
 // RUN: FileCheck -check-prefix=CHECK-NOINLINE %s
 // RUN: %clang -DINLINE="__attribute__((noinline))" -target x86_64-linux-gnu \
-// RUN: -O0 -o - -c -std=c99 -xc %s | llvm-nm - 2>&1 | \
+// RUN: -O0 -o - -c -std=c99 -xc %s | llvm-nm -Xany - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-NOINLINE %s
 
 // RUN: %clang_cc1 -DINLINE="static" -triple x86_64-unknown-linux-gnu -o - \
 // RUN: -emit-interface-stubs -std=c99 -xc %s | \
 // RUN: FileCheck -check-prefix=CHECK-STATIC %s
 // RUN: %clang -DINLINE="static" -target x86_64-linux-gnu -O0 -o - -c \
-// RUN:   -std=c99 -xc %s | llvm-nm - 2>&1 | count 0
+// RUN:   -std=c99 -xc %s | llvm-nm -Xany - 2>&1 | count 0
 
 // CHECK-GNU-DAG: foo
 // CHECK-GNU-DAG: foo.var
@@ -52,7 +52,7 @@
 // RUN: -emit-interface-stubs \
 // RUN: -std=gnu89 -xc %s | FileCheck -check-prefix=CHECK-SYMBOLS %s
 // RUN: %clang -DINLINE=inline -target x86_64-linux-gnu -o - \
-// RUN: -c -std=gnu89 -xc %s | llvm-nm - 2>&1 | \
+// RUN: -c -std=gnu89 -xc %s | llvm-nm -Xany - 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
 
 // CHECK-TAPI-DAG: foo", Type: Func }
Index: clang/test/InterfaceStubs/function-template-specialization.cpp
===
--- clang/test/InterfaceStubs/function-template-specialization.cpp
+++