[PATCH] D29011: [IR] Add Freeze instruction

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune marked 5 inline comments as done.
aqjune added inline comments.


================
Comment at: include/llvm/IR/IRBuilder.h:1715
+  /// the new value.
+  Value *CreateFreezeAtDef(Value *Arg, Function *F, const Twine &Name = "",
+                           bool replaceAllUses = true) {
----------------
filcab wrote:
> This is not being used. Maybe submit in a later patch, with a use for it?
Hello. Thanks for your comment.
This function is going to be used in https://reviews.llvm.org/D29016, and this patch is important because it may reduce the number of unnecessarily inserted freezes.


================
Comment at: include/llvm/IR/PatternMatch.h:931
+template <typename OpTy>
+inline FreezeClass_match<OpTy> m_Freeze(const OpTy &Op) {
+  return FreezeClass_match<OpTy>(Op);
----------------
You're right - it is not used right now.
However every other values has its own match function currently, so I just followed convention..
If you think this is redundant now, I'll erase this.


================
Comment at: lib/Analysis/ValueTracking.cpp:3917
+    return false;
+  }
+
----------------
There's no plan to add other cases now.
I added TODOs, as you recommended.


================
Comment at: lib/AsmParser/LLParser.cpp:5914
+  if (!Op->getType()->isIntegerTy())
+    return Error(Loc,"cannot freeze non-integer type");
+
----------------
I added 3 test cases which checks whether this error message is printed out.


================
Comment at: lib/AsmParser/LLParser.cpp:5916
+
+  Inst = new FreezeInst(Op);
+  return false;
----------------
I removed the second argument.


https://reviews.llvm.org/D29011