Page MenuHomePhabricator

[X86] Break the loop in LowerReturn into 2 loops.
ClosedPublic

Authored by craig.topper on Jan 13 2020, 4:05 PM.

Details

Summary

I believe for STRICT_FP I need to use a STRICT_FP_EXTEND for the extending to f80 for returning f32/f64 in 32-bit mode when SSE is enabled. The STRICT_FP_EXTEND node requires a Chain. I need to get that node onto the chain before any CopyToRegs are emitted. This is because all the CopyToRegs are glued and chained together. So I can't put a STRICT_FP_EXTEND on the chain between the glued nodes without also glueing the STRICT_ FP_EXTEND.

This patch moves all the extend creation to a first pass and then creates the copytoregs and fills out RetOps in a second pass.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 13 2020, 4:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2020, 4:05 PM
Herald added a subscriber: hiraditya. · View Herald Transcript

Do we really need to distinguish STRICT_FP_EXTEND and FP_EXTEND for a return? Currently the STRICT nodes are used to avoid to be optimized out or reordered. But I think neither occurs for a return.

This FP_EXTEND will be turned into a SSE store and an X87 load. That X87 load may generate an exception so we should have the fpexcept property on it. That requires a STRICT_FP_EXTEND.

Yeah, it's useful and clear to use a STRICT_FP_EXTEND. Anyway, I think we can also achieve it by getting the function's attribute or propagating the fpexcept to FP_EXTEND.

Yeah, it's useful and clear to use a STRICT_FP_EXTEND. Anyway, I think we can also achieve it by getting the function's attribute or propagating the fpexcept to FP_EXTEND.

Only nodes that return true for isTargetStrictFPOpcode() or isStrictFPOpcode() can have a fpexcept flag. Because the flag is really nofpexcept and none strict nodes are just assumed to always be nofpexcept.

rnk added a comment.Jan 24 2020, 1:30 PM

From the commit message, this sounds like it is fixing a bug. Can it be tested? Did you have a test already, or did you forget to add it?

Otherwise, code looks good.

craig.topper edited the summary of this revision. (Show Details)Jan 24 2020, 1:47 PM
In D72665#1839600, @rnk wrote:

From the commit message, this sounds like it is fixing a bug. Can it be tested? Did you have a test already, or did you forget to add it?

Otherwise, code looks good.

This patch was intended as an NFC rework. The use of STRICT_FP_EXTEND in strictfp functions will be a separate patch.

rnk accepted this revision.Jan 24 2020, 1:51 PM

lgtm

This revision is now accepted and ready to land.Jan 24 2020, 1:51 PM
This revision was automatically updated to reflect the committed changes.