[PATCH] D29014: [SelDag] Implement FREEZE node

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added inline comments.


================
Comment at: include/llvm/CodeGen/ISDOpcodes.h:180
 
+    // FREEZE - A freeze node
+    FREEZE,
----------------
Please add more documentation here.
In particular, a very short form of the explanation from SelectionDAGISel::Select_FREEZE() about the difference in semantics between FREEZE and UNDEF would be appropriate.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2238
+  // MachineInstr.
+  // LLVM translates an UNDEF node into multiple IMPLICIT_DEF
+  // instructions (in MachineInstr) if the UNDEF has multiple uses.
----------------
I'm a bit confused about this. The function above selects UNDEF to IMPLICIT_DEF.
Where do we do the duplication? If we duplicate UNDEFs on the DAG level, then everything's already fine, and you can just select a FREEZE directly to IMPLICIT_DEF.
If we're duplicating IMPLICIT_DEFs, then this is a problem regardless of what you do here, if the freeze ends up being an IMPLICIT_DEF. Or are you saying we're duplicating IMPLICIT_DEFs in the DAG, but not in MI? I'd find that surprising.

(Note that the documentation for IMPLICIT_DEF just says that "this is the MachineInstr-level equivalent of undef." If we're not allowed to duplicate IMPLICIT_DEFs and they have a single defined value, that should be corrected.)


https://reviews.llvm.org/D29014