stevewan 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"); ---------------- jasonliu wrote: > Xiangling_L wrote: > > Is there any reason to use llvm_unreachable here? I think we should use > > 'assertion' instead here: > > > > ``` > > assert((IsArch32Bit || IsArch64Bit) && "..."); > > ``` > IsArch64Bit used only in the assertion could cause warning when the assertion > is turned off. Jason has provided a good point why `llvm_unreachable` was preferred here. Other than that, I believe the two are fairly interchangeable in this particular case. That said, I'm leaning towards keeping `llvm_unreachable`, but definitely add more comment if you have good reasons for using `assert`. Thanks! ================ Comment at: clang/lib/Driver/ToolChains/AIX.cpp:54 + } else { + assert(Output.isNothing() && "Invalid output."); + } ---------------- Xiangling_L wrote: > I am not sure, if we compile with assertion off, does this extra 'else' {} > have any side effect? As per my tests with assertion-off build, I found no side effect and/or unexpected behaviour caused by this. There was no warning or anything unexpected that would've not appeared when assertion is on. ================ Comment at: clang/lib/Driver/ToolChains/AIX.cpp:74 + if (Args.hasArg(options::OPT_pg)) + crt0 = IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o"; + // Enable profiling when "-p" is specified ---------------- hubert.reinterpretcast wrote: > For 32-bit mode, there is a "reentrant" variant for when `-pthread` or > `-pthreads` is specified. The `crt0_r.o` has become a symlink to `crt0.o`, we don't need to add extra handling for 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