Page MenuHomePhabricator

WebAssembly: implement comparison.
ClosedPublic

Authored by jfb on Aug 10 2015, 5:35 PM.

Details

Summary

Some of the FP comparisons (ueq, one, ult, ule, ugt, uge) are currently broken, I'll fix them in a follow-up.

Diff Detail

Repository
rL LLVM

Event Timeline

jfb updated this revision to Diff 31749.Aug 10 2015, 5:35 PM
jfb retitled this revision from to WebAssembly: implement comparison..
jfb updated this object.
jfb added a reviewer: sunfish.
jfb added a subscriber: llvm-commits.
sunfish accepted this revision.Aug 10 2015, 7:05 PM
sunfish edited edge metadata.
sunfish added inline comments.
lib/Target/WebAssembly/WebAssemblyInstrFloat.td
27 ↗(On Diff #31749)

The unordered comparisons (except "une") will be legalized. The optimization to do here is to fold the "don't care" comparisons to supported comparisons.

29 ↗(On Diff #31749)

The NE operator is "une" rather than "one".

33 ↗(On Diff #31749)

This patch lacks tests for the floating point comparisons.

This revision is now accepted and ready to land.Aug 10 2015, 7:05 PM
jfb updated this object.Aug 10 2015, 7:54 PM
jfb edited edge metadata.
jfb updated this revision to Diff 31761.Aug 10 2015, 8:50 PM
  • Add (broken) f32 test, trying to expand unsupported floating-point comparisons.
jfb marked an inline comment as done.Aug 10 2015, 8:54 PM
jfb added inline comments.
lib/Target/WebAssembly/WebAssemblyInstrFloat.td
31–37 ↗(On Diff #31761)

I'm not sure I get what you mean about the "don't care" comparisons. You mean TRUE and FALSE?

33 ↗(On Diff #31761)

I've been reading the DAG legalization code... I'm not sure I get it yet. I uploaded an intermediate update which is broken for now (and the CHECK are wrong, because I haven't settled on which expansions are needed)... I'm not sure I get the expansion order. Right now it says it can't select ueq, but it should have been expanded.

37 ↗(On Diff #31761)

I'll add f64 when f32 works.

jfb updated this revision to Diff 31840.Aug 11 2015, 11:27 AM
jfb marked an inline comment as done.
  • Fix typo.
jfb updated this revision to Diff 31843.Aug 11 2015, 11:44 AM
  • Use the same 3-operand setcc+cond for FP comparisons as for int. It shouldn't change anything because of TargetSelectionDAG.td:910, but this is confusing...
jfb updated this revision to Diff 31857.Aug 11 2015, 2:00 PM
  • Add (broken) f32 test, trying to expand unsupported floating-point comparisons.
  • Fix typo.
  • Use the same 3-operand setcc+cond for FP comparisons as for int. It shouldn't change anything because of TargetSelectionDAG.td:910, but this is confusing...
  • Remove currently broken FP comparisons (ueq, one, ult, ule, ugt, uge), so I can commit this and fix the broken ones separately.
jfb updated this object.Aug 11 2015, 2:01 PM

I'll commit this patch since the functionality it adds is correct, and I'll address broken FP comparisons in a follow-up.

This revision was automatically updated to reflect the committed changes.