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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
