This is an archive of the discontinued LLVM Phabricator instance.

[AIX] change "llvm-nm" to "env OBJECT_MODE=any llvm-nm" in clang/test for AIX OS
ClosedPublic

Authored by DiggerLin on Sep 20 2022, 7:11 AM.

Details

Summary

since default object mode of llvm-nm is change to -X32 (from default -Xany) in patch https://reviews.llvm.org/D132494.In order not effect the test cases in clang/test we need to change "llvm-nm" to "env OBJECT_MODE=any llvm-nm" in clang/test for AIX OS

Diff Detail

Event Timeline

DiggerLin created this revision.Sep 20 2022, 7:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 7:11 AM
DiggerLin requested review of this revision.Sep 20 2022, 7:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2022, 7:11 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

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

DiggerLin added a comment.EditedSep 20 2022, 7:23 AM

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"

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?

DiggerLin added a comment.EditedSep 20 2022, 7:37 AM

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

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.

daltenty added a comment.EditedSep 20 2022, 7:50 AM

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 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.

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.

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.

DiggerLin updated this revision to Diff 461589.Sep 20 2022, 9:43 AM
DiggerLin updated this revision to Diff 461598.Sep 20 2022, 9:50 AM

address comment

DiggerLin retitled this revision from [AIX] change the clang tests with llvm-nm -Xany to [AIX] change "llvm-nm" to "env OBJECT_MODE=any llvm-nm" in clang/test for AIX OS.Sep 20 2022, 10:26 AM
DiggerLin edited the summary of this revision. (Show Details)
daltenty accepted this revision.Sep 20 2022, 10:50 AM

LGTM to unblock the AIX buildbots. We can follow up on a separate env later if desired.

This revision is now accepted and ready to land.Sep 20 2022, 10:50 AM
jhenderson accepted this revision.Sep 21 2022, 12:02 AM

LGTM, with nit.

clang/test/lit.cfg.py
280