This is an archive of the discontinued LLVM Phabricator instance.

[mlir][LLVMIR] Fix incorrect llvm.fcmp result type
ClosedPublic

Authored by myhsu on Sep 22 2022, 9:27 AM.

Details

Summary

Similar to D127536, if any of the operands for FCmpOp is a vector, returns a vector<Nxi1> rather than an i1 type result.

Diff Detail

Build Status
Buildable 188195

Event Timeline

myhsu created this revision.Sep 22 2022, 9:27 AM
myhsu requested review of this revision.Sep 22 2022, 9:27 AM
Mogball added inline comments.Sep 23 2022, 9:42 AM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
890

does llvm.fmp do implicit broadcasting?

myhsu added inline comments.Sep 26 2022, 2:37 PM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
890

I just checked LLVM IR and LLVM Dialect's semantic and surprisingly found that neither llvm.icmp nor llvm.fcmp (and their LLVM IR counterparts) do implicit broadcasting: LLVM IR simply says that operands under comparison should have identical types. Let me cleanup the code here.

What I'm more curious is why did the original ICmpOp::build do implicit broadcasting at the first place?

myhsu updated this revision to Diff 463036.Sep 26 2022, 3:04 PM
myhsu marked an inline comment as done.

Do not broadcast when the operand types are different

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
890

What I'm more curious is why did the original ICmpOp::build do implicit broadcasting at the first place?

nvm, it was me that first did implicit broadcasting for ICmpOp::build in D127536 for reason I cannot remember now.

ftynse accepted this revision.Sep 27 2022, 7:02 AM
This revision is now accepted and ready to land.Sep 27 2022, 7:02 AM
This revision was automatically updated to reflect the committed changes.