MaskRay added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:440
+      StringRef Val = A->getValue();
+      if (Val == "atomic" || Val == "prefer-atomic")
+        CmdArgs.push_back("-latomic");
----------------
qiongsiwu1 wrote:
> This check here is copied from 
> https://github.com/llvm/llvm-project/blob/77596e6b167bf0a5efa790597d6b75ac5e685b55/clang/lib/Driver/ToolChains/Clang.cpp#L867,
>  and I don't think it is good to copy this particular code over because we 
> are doing the same check twice at different places and the code can go out of 
> sync. 
> 
> May I get some suggestions to avoid duplicating the code? Thanks! 
This seems ok to me. You can also use `!= "single"` instead.

`getLastArgNoClaim` is probably slightly better.


================
Comment at: clang/test/Driver/fprofile-update.c:17
+// AIX specific tests
+// RUN: %if system-aix %{ %clang -### %s -fprofile-generate 
-fprofile-update=atomic 2>&1 | FileCheck %s --check-prefix=AIX %}
+// RUN: %if system-aix %{ %clang -### %s -fprofile-generate 
-fprofile-update=prefer-atomic 2>&1 | FileCheck %s --check-prefix=AIX %}
----------------
The idiom is to use `--target=...`. This driver decision is not dependent on 
what OS the host runs, so `%if system-aix` should be avoided.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159137

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

Reply via email to