[PATCH] D75702: [PowerPC32] Fix the setcc unexpected expansion problem

Sean Fertile via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 5 13:12:30 PST 2020


sfertile added a comment.

I get why you named the test `ppc32-setcc-expansion.ll` but to a casual reader its definitely unclear what a setcc has to do with  this code. I would suggest  'ppc32-i64-to-float-conv.ll', and have a comment explaining how the lowering for the conversion generates a setcc which caused a crash under certain conditions.

Also does the IR that inspired this patch work when CR-bits are disabled (if we can disable them)? If so we should verify it works after the change also.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8119
                          Cond, DAG.getConstant(1, dl, MVT::i64));
-      Cond = DAG.getSetCC(dl, MVT::i32,
-                          Cond, DAG.getConstant(1, dl, MVT::i64), ISD::SETUGT);
+      Cond = DAG.getSetCC(dl, Subtarget.useCRBits() ? MVT::i1 : MVT::i32, Cond,
+                          DAG.getConstant(1, dl, MVT::i64), ISD::SETUGT);
----------------
You are on the right track but I think `getSetCCResultType` is the interface we want to use to get the proper result type. FWIW it will be the same behavior but somewhat self documenting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75702





More information about the llvm-commits mailing list