Xiangling_L added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:35
+ // Only support 32 and 64 bit
+ if (!IsArch32Bit && !IsArch64Bit)
+ llvm_unreachable("Unsupported bit width value");
----------------
Is there any reason to use llvm_unreachable here? I think we should use
'assertion' instead here:
```
assert((IsArch32Bit || IsArch64Bit) && "...");
```
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:54
+ } else {
+ assert(Output.isNothing() && "Invalid output.");
+ }
----------------
I am not sure, if we compile with assertion off, does this extra 'else' {} have
any side effect?
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:70
+
+ if (!Args.hasArg(options::OPT_nostdlib)) {
+ const char *crt0 = nullptr;
----------------
line 38 and line 70 use the same query, should they be put together? Or is
there any exact order we should follow to push args into 'CmdArgs'?
================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:93
+ if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+ // Support POSIX threads if "-pthreads" or
+ // "-pthread" is present
----------------
One line of comment can be <= 80 characters.
================
Comment at: clang/lib/Driver/ToolChains/AIX.h:33
+ const char *LinkingOutput) const override;
+};
+} // end namespace aix
----------------
An extra blank line preferred below.
================
Comment at: clang/lib/Driver/ToolChains/AIX.h:43
+ const llvm::opt::ArgList &Args);
+ ~AIX() override;
+
----------------
Since we are not doing anything special in AIX toolchain destructor, seems like
we don't need to override it?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68340/new/
https://reviews.llvm.org/D68340
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
