This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Replace untested code with assert
ClosedPublic

Authored by frasercrmck on May 23 2022, 11:39 PM.

Details

Summary

We found untested code where negative frame indices were ostensibly
handled despite it being in a block guarded by !MFI.isFixedObjectIndex.

While the implementation of MachineFrameInfo::isFixedObjectIndex
suggests this is possible (i.e., if a frame index was more negative - less than the
number of fixed objects), I couldn't find any test in tree -- for any
target -- where a negative frame index wasn't also a fixed object
offset. I couldn't find a way of creating such a object with the
public MachineFrameInfo creation APIs. Even
MachineFrameInfo::getObjectIndexBegin starts counting at the negative
number of fixed objects, so such frame indices wouldn't be covered by
loops using the provided begin/end methods.

Given all this, an assert that any object encountered in the block is
non-negative seems reasonable.

Diff Detail

Event Timeline

frasercrmck created this revision.May 23 2022, 11:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2022, 11:39 PM
frasercrmck requested review of this revision.May 23 2022, 11:39 PM
frasercrmck edited the summary of this revision. (Show Details)May 23 2022, 11:40 PM
StephenFan accepted this revision.May 24 2022, 9:04 AM
This revision is now accepted and ready to land.May 24 2022, 9:04 AM

Pushed to our internal testing system to see what happen, will back to you once I got result :)

kito-cheng accepted this revision.May 24 2022, 6:36 PM

Green light on testing :)

This revision was landed with ongoing or failed builds.May 24 2022, 9:14 PM
This revision was automatically updated to reflect the committed changes.

Green light on testing :)

Thanks very much for doing this, it's a big help!