[PATCH] D80714: [StatepointLowering] Handle UNDEF gc values.

[Philip Reames via Phabricator via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/required changes applied before commit.  If you disagree on any point, further review needed and LGTM does not apply.

Also, make sure you update your commit message before landing.  The current description is out of sync with the patch.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:232
 
+  if (Incoming.getOpcode() == ISD::UNDEF)
+    return;
----------------
Use Incomng.isUndef(), add to list in following return since the comment applies as well.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:397
+    // true today.  If it stops being true, this assert will fail.
+    assert(Incoming.getValueType().getSizeInBits() <= 64 &&
+           "unexpected wide undef");
----------------
Please remove the assert by folding it into an additional clause in the condition.

e.g. if (Incoming is undef and <= 64 bits)

Do the same below.

Vector undefs are legal and shouldn't crash.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:401
+    // value so that uses of undef by the consumer of the stackmap is
+    // easily recognized.
+    pushStackMapConstant(Ops, Builder, 0xFEFEFEFE);
----------------
Add to comment.  (This is legal since the compiler is always allowed to chose an arbitrary value for undef.)


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:1024
 
+  if (SD.getOpcode() == ISD::UNDEF) {
+    // We assume that vector undef's never get here.  That appears to be
----------------
Use SD.isUndef


================
Comment at: llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp:1028
+    assert(SD.getValueType().getSizeInBits() <= 64 && "unexpected wide undef");
+    // Relocating an undef doesn't make sense.  Replace this with an constant
+    // value which is unlikely to be a valid pointer to make isolating uses
----------------
After removing the assert, replace comment with:
Lowering relocate(undef) as arbitrary constant.  Current constant value is chosen such that it's unlikely to be a valid pointer.   


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80714/new/

https://reviews.llvm.org/D80714