Page MenuHomePhabricator

[RISCV] replace unuseful emergency spill slot test with a mir test
Needs ReviewPublic

Authored by StephenFan on Feb 23 2021, 5:07 AM.

Details

Summary

It seems that the original emergency spill slot test doesn't use the emergency spill slot. So I replaced it with a mir test

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 debian > Flang.Semantics::resolve102.f90
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/test_errors.sh /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/resolve102.f90 /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/test/Semantics/Output/resolve102.f90.tmp /mnt/disks/ssd0/agent/llvm-project/build/bin/f18 -intrinsic-module-directory /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/include/flang

Event Timeline

StephenFan created this revision.Feb 23 2021, 5:07 AM
StephenFan requested review of this revision.Feb 23 2021, 5:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 5:07 AM

From some quick testing, it seemed that you could trim this test down a bit.
You're also adding # REQUIRES: asserts but not actually checking for any debug output.
Also, it would be nice to add a comment documenting the logic of this test, unless new CHECKs of the debug output make it more obvious.

Address @luismarques 's comment

Yes, I referenced this out-of-reach-emergency-slot.mir test. But with a stack object that has a out-of-range size. And I think my use-emergency-spill-slot.mir test and out-of-reach-emergency-slot.mir test are a little repeated. So should I keep my use-emergency-spill-slot.mir test or delete it?

Yes, I referenced this out-of-reach-emergency-slot.mir test. But with a stack object that has a out-of-range size. And I think my use-emergency-spill-slot.mir test and out-of-reach-emergency-slot.mir test are a little repeated. So should I keep my use-emergency-spill-slot.mir test or delete it?

Oh sorry I failed to notice you also adjusted maxAlignment to something sensible. Both look very similar but we could argue they are testing different things, so I think this can be useful.