[PATCH] D18291: Add IntrOnlyWrite intrinsic property

[Nicolai Hähnle via llvm-commits]

nhaehnle marked an inline comment as done.

================
Comment at: utils/TableGen/CodeGenIntrinsics.h:67
@@ +66,3 @@
+      MR_Write = 4,
+      MR_ReadWrite = MR_Read | MR_Write,
+
----------------
joker.eph wrote:
> 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).
The intention is to be analogous to the FunctionModRefBehavior in AliasAnalysis. The values of the bits are different to preserve the ordering of the possible numerical values.

I'm changing MR_Read/Write to MR_Ref/Mod to make this clearer, and putting the bits into their own enum.

================
Comment at: utils/TableGen/IntrinsicEmitter.cpp:603
@@ -602,2 +602,3 @@
       switch (intrinsic.ModRef) {
+      default: PrintFatalError("unhandled intrinsic.ModRef");
       case CodeGenIntrinsic::NoMem:
----------------
joker.eph wrote:
> Don't you get a warning for fully covered switch?
Not originally, because of the non-sensical possibility of MR_Anywhere without MR_Read/Ref or MR_Write/Mod. With the new enum organization it can indeed be removed. (I didn't get the warning even then, perhaps that's a gcc vs. clang thing.)


http://reviews.llvm.org/D18291