[PATCH] D65088: [AMDGPU][RFC] New llvm.amdgcn.ballot intrinsic

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4181-4182
+    if (ConstSrc->isNullValue())
+      // Optimization: ballot(0) returns all zeroes.
+      return DAG.getConstant(0, DL, VT);
+    if (ConstSrc->isOne()) {
----------------
Should also do this in instcombine, as icmp/fcmp already do


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4196
+                              DAG.getCondCode(ISD::SETNE));
+  return DAG.getZExtOrTrunc(SetCC, DL, VT);
+}
----------------
This is already the right type, so this should be unnecessary


================
Comment at: llvm/test/CodeGen/AMDGPU/ballot.ll:4
+
+declare i64 @llvm.amdgcn.ballot(i1)
+
----------------
This needs the .i64 for the mangling, I'm surprised this works


================
Comment at: llvm/test/CodeGen/AMDGPU/ballot.ll:49
+  ret i64 %ballot
+}
----------------
Can you add some different imp and fcmp sources?

Also need a separate wave32 test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65088