Page MenuHomePhabricator

[RISCV] Split f64 undef into two i32 undefs

Authored by pcwang-thead on Jan 26 2022, 1:21 AM.



So that no store instruction will be generated.

Diff Detail

Event Timeline

pcwang-thead created this revision.Jan 26 2022, 1:21 AM
pcwang-thead requested review of this revision.Jan 26 2022, 1:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2022, 1:21 AM

This could do with some tests to show the effect on codegen.


"since the ProcessImplicitDefs pass will do it for us". I don't think referring to the DEBUG_TYPE is as helpful.

Address comment.

pcwang-thead marked an inline comment as done.Jan 26 2022, 5:01 AM

This would be more useful as a DAG combine in performDAGCombine. That would allow the integer undefs to be further simplified.

Change to DAG combine.

pcwang-thead retitled this revision from [RISCV] Split f64 undef into two integer undefs to [RISCV] Split f64 undef into two i32 undefs.Jan 26 2022, 7:09 PM
craig.topper added inline comments.Jan 26 2022, 7:22 PM
269 ↗(On Diff #403469)

I thought the issue showed up with the ilp32 abi and the D extension, not the ilp32d abi? Please check this test case in this test file fails without the RISCVISelLowering.cpp change.

Update tests.

pcwang-thead marked an inline comment as done.Jan 26 2022, 8:32 PM

Can we put the new test case in double-calling-conv.ll instead? I think that it already has the right command line.

Move test to double-calling-conv.ll.

Given that the description says "so that no store will be generated" I think it'd be helpful to pre-commit the test case so that this patch shows the improvement. I tested it locally and see:

define double @func_return_double_undef() nounwind {
; RV32IFD-LABEL: func_return_double_undef:
; RV32IFD:       # %bb.0:
; RV32IFD-NEXT:    addi sp, sp, -16
; RV32IFD-NEXT:    fsd ft0, 8(sp)
; RV32IFD-NEXT:    lw a0, 8(sp)
; RV32IFD-NEXT:    lw a1, 12(sp)
; RV32IFD-NEXT:    addi sp, sp, 16
; RV32IFD-NEXT:    ret
  ret double undef

Other than that LGTM.

asb accepted this revision.Jan 27 2022, 7:53 AM

Thanks, LGTM.

This revision is now accepted and ready to land.Jan 27 2022, 7:53 AM
This revision was landed with ongoing or failed builds.Feb 7 2022, 9:43 PM
This revision was automatically updated to reflect the committed changes.