[PATCH] D69620: Add AIX assembler support

Hubert Tong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 1 09:57:23 PST 2019


hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM with minor change request to a comment. Thanks.



================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:68
+  if (Inputs.size() != 1)
+    llvm_unreachable("Invalid number of input files.");
+  const InputInfo &II = Inputs[0];
----------------
stevewan wrote:
> hubert.reinterpretcast wrote:
> > `llvm_unreachable` is not the right solution if this can be reached by "user error". We should produce a proper error message.
> One risky condition I can think of is the user passes in multiple assembly sources to the driver, which may lead to multiple assembler inputs. To verify how the driver handles such a case, I added a new test into `aix-as.c` below, whose results suggested that this is okay because the driver would invoke `as` for each and every input files respectively. Looking into the code, the driver would construct an action list for each input files individually, which again matches the behaviour we observed in the testing results. That said, I believe the `llvm_unreachable` here is indeed not reachable by "user errors" like this, and if it triggers, it's likly an internal error. 
Thanks. Can we indicate that we are expecting the driver to invoke `as` separately for each assembler source file in the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620





More information about the cfe-commits mailing list