[PATCH] D144189: [AIX][CodeGen] Storage Locations for Constant Pointers

[Qiongsi Wu via Phabricator via llvm-commits]

qiongsiwu1 added inline comments.


================
Comment at: llvm/tools/llc/llc.cpp:513
+      if (!Options.DataSections)
+        reportError("-mroptr option must be used with -data-sections",
+                    InputFilename);
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > See https://reviews.llvm.org/D144190#inline-1406700 for why having this diagnostic in `llc` may not be the best.
> Since the back end now errors out reliably (`report_fatal_error`), the comment linked above for the other patch implies that we can remove this `llc` diagnostic in this patch. Then we can land this patch first.
> Since the back end now errors out reliably (`report_fatal_error`), the comment linked above for the other patch implies that we can remove this `llc` diagnostic in this patch. Then we can land this patch first.

It seems that the `report_fatal_error` mechanism works more like an assert. `llc` crashes when encountering a `report_fatal_error` function. It is probably not ideal that we crash when `-data-sections=false` is passed through the command line. Should we keep this `llc` check? I am inclined to do so because I don't think we should allow `llc` to crash. That said, if the crashing behaviour is acceptable, I will remove the `-data-sections=false` test case, and remove this check. Alternatively, there may be other error reporting mechanisms in the backend that I am not aware of that can generate a valid error message without crashing. If there are, I will adopt them so we can report the error in the backend, and not crash. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144189