[PATCH] D66724: [AIX]Emit function descriptor csect in assembly

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 27 07:12:39 PDT 2019


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/MCSectionXCOFF.cpp:27
+  default:
+    llvm_unreachable("Unsupported storage-mapping class");
+  }
----------------
sfertile wrote:
> xingxue wrote:
> > jasonliu wrote:
> > > sfertile wrote:
> > > > A `report_fatal_error` is probably more appropriate until we have added all the storage mapping classes.
> > > This marked done, but haven't really addressed. 
> > In my committed patch https://reviews.llvm.org/D66154, the llvm_unreachable() here was replaced with assert() under Hubert's suggestion, which makes sense to me.
> I'm OK with `assert` as well.
The other patch did not invoke undefined behaviour if the assertion failed. This patch does (there is no `return` statement following the `llvm_unreachable`). `report_fatal_error` makes sense. Also, we should indicate "Unhandled storage-mapping class." "Unsupported" has proven itself to be confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66724





More information about the llvm-commits mailing list