[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