[PATCH] D81711: [SDAG] Add new AssertAlign ISD node.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5191-5194
+  // There's no need to assert on a byte-aligned pointer. All pointers are at
+  // least byte aligned.
+  if (A < 2)
+    return Val;
----------------
arsenm wrote:
> arsenm wrote:
> > This check is unnecessary, a lower alignment isn't even representable in Align because it stores the log2 of the alignment
> Nevermind, I misread this. I think it would be clearer as if (A == Align(1)) return Val
I think == Align(1) would be clearer


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1634
+      Addr.getOperand(0).getOpcode() == ISD::BUILD_VECTOR) {
+    // A complicated pattern as we split 64-bit `or` earlier.
+    SDValue Lo = Addr.getOperand(0).getOperand(0);
----------------
Can you add a dag style comment for the pattern this matches? This could also use some early returns


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81711