[PATCH] D152924: [libLTO][AIX] Respect `-f[no]-integrated-as` on AIX

Tue Jul 11 12:56:54 PDT 2023

MaskRay accepted this revision.
MaskRay added a comment.

Some nits about testing, otherwise LG

In D152924#4490950 <https://reviews.llvm.org/D152924#4490950>, @qiongsiwu1 wrote:

> In D152924#4490581 <https://reviews.llvm.org/D152924#4490581>, @MaskRay wrote:
>
>> We have `TargetOptions::DisableIntegratedAS` (your llc.cpp change). Do you know why it is not feasible for the places you want to check?
>
> I am checking/using `TargetOptions::DisableIntegratedAS` in `LTOCodeGenerator::useAIXSystemAssembler()` to determine if the system assembler should be used. `NoIntegratedAssembler` is moved from an `llc` only option to a codegen option so LTO can use it as well.

Sorry, my question was not clear. Anyway, I figured it out.
We need to pass `lto::Config` to `lto::backend` with the correct `CGFileType` information.  `TargetOptions::DisableIntegratedAS` is already present, but we don't initialize it in `llvm/lib/CodeGen/CommandFlags.cpp`.
Thanks. This looks good to me.

================
Comment at: llvm/test/tools/llvm-lto/aix-sys-as.ll:1
+; REQUIRES: system-aix
+; RUN: llvm-as < %s > %t1
----------------
I suggest that you merge this test into aix.ll.

You can use

```
RUN: %if system-aix %{ ... %}
```
to only run a RUN line when the lit feature `system-aix` is available.

================
Comment at: llvm/test/tools/llvm-lto/aix.ll:3
 ; RUN: llvm-as < %s > %t1
 ; RUN: llvm-lto %t1 | FileCheck %s

----------------
It seems that with or without `-o`, the behavior may be different?

In some lit configurations (e.g., Google's internal lit runner), the current working directory if a test invocation is read-only. We need to ensure that the directory is writable.

You can do
```
RUN: rm -rf %t && mkdir %t && cd %t`
RUN: llvm-as < %s -o a.bc
RUN: llvm-lto a.bc
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152924