[PATCH] D18291: Add IntrOnlyWrite intrinsic property

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 1 14:51:21 PDT 2016


joker.eph added a comment.

Looks OK in principle, but please see inline comments.


================
Comment at: utils/TableGen/CodeGenIntrinsics.h:67
@@ +66,3 @@
+      MR_Write = 4,
+      MR_ReadWrite = MR_Read | MR_Write,
+
----------------
I found it confusing that these are used to define the enum value below and also separately in the code. Maybe use a separate enum?
Also documenting the lattice would be nice.
The individual value meaning might be worth a comment, for instance MR_Read is *read through argument only* from what I understand below (and the name does not convey this).

================
Comment at: utils/TableGen/IntrinsicEmitter.cpp:603
@@ -602,2 +602,3 @@
       switch (intrinsic.ModRef) {
+      default: PrintFatalError("unhandled intrinsic.ModRef");
       case CodeGenIntrinsic::NoMem:
----------------
Don't you get a warning for fully covered switch?


http://reviews.llvm.org/D18291





More information about the llvm-commits mailing list