This is an archive of the discontinued LLVM Phabricator instance.

[LegalizeTypes] Mutate strict float operations to non-strict when softening
AbandonedPublic

Authored by john.brawn on Nov 21 2019, 10:28 AM.

Details

Summary

Currently if a strict float operation is softened we get an assertion failure due to strict float operations not being handled. Fix this in the simplest way be first converting to the non-strict version of the operation.

Diff Detail

Event Timeline

john.brawn created this revision.Nov 21 2019, 10:28 AM

I was going to start working on fixing this correctly next week. Can you wait for that?

I was going to start working on fixing this correctly next week. Can you wait for that?

Sure. There's also a problem in SelectionDAGLegalize::ConvertNodeToLibcall where not all strict opcodes are handled (you can see errors caused by this if you run the CodeGen/ARM/fp-intrinsics.ll test with -mattr=fp-armv8sp, as that means we have to convert double-precision operations to libcalls), are you going to be looking at that as well or should I take a look at that? I've had a brief look so far and doing the obvious thing (add e.g. case ISD::STRICT_FADD to where we currently have ISD::FADD) causes failures in the X86 tests, so it looks like it'll need to take some work to fix.

It looks like there's also some stuff in the ARM target (some custom lowering) that needs fixing. I think what I'll do is open a separate review for adding just the test (XFAILed for now), then work on fixing the stuff in the ARM target.

john.brawn abandoned this revision.Nov 22 2019, 7:56 AM

D70599 just adds the test, and abandoning this change.

I was going to start working on fixing this correctly next week. Can you wait for that?

Sure. There's also a problem in SelectionDAGLegalize::ConvertNodeToLibcall where not all strict opcodes are handled (you can see errors caused by this if you run the CodeGen/ARM/fp-intrinsics.ll test with -mattr=fp-armv8sp, as that means we have to convert double-precision operations to libcalls), are you going to be looking at that as well or should I take a look at that? I've had a brief look so far and doing the obvious thing (add e.g. case ISD::STRICT_FADD to where we currently have ISD::FADD) causes failures in the X86 tests, so it looks like it'll need to take some work to fix.

I believe I fixed that issue yesterday.