This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use AdjustInstrPostInstrSelection to insert a FRM dependency for scalar FP instructions with dynamic rounding mode.
ClosedPublic

Authored by craig.topper on Dec 10 2021, 2:46 PM.

Details

Summary

In order to support constrained FP intrinsics we need to model FRM
dependency. Whether or not a instruction uses FRM is based on a 3
bit field in the instruction. Because of this we can't add
'Uses = [FRM]' to the tablegen descriptions.

This patch examines the immediate after isel and adds an implicit
use of FRM. This idea came from Roger Ferrer Ibanez.

Other ideas:
We could be overly conservative and just pretend all instructions with
frm field read the FRM register. Or we could have pseudoinstructions
for CodeGen with rounding mode.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 10 2021, 2:46 PM
craig.topper requested review of this revision.Dec 10 2021, 2:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2021, 2:46 PM
Herald added a subscriber: MaskRay. · View Herald Transcript

As it stands the implementation is a bit assuming, relying on it only ever being called for floating point instructions (or those that don't have a funct3), but there are other uses, so should also be checking getOperandType is OpTypes::frmarg? Could even ditch the name lookup entirely and just iterate over the use operands, don't know what's better.

llvm/lib/Target/RISCV/RISCVInstrInfo.h
24

Combine?

Regarding something like using pseudos instead, you could, but that's a lot more effort, so if this works it's probably better. If it doesn't work then it's easy enough to switch to something more involved, this is pretty non-invasive.

Rename funct3 in the underlying tablegen classes to frm so that we can call
it frm in the lookup. This is what I wanted to do all along, but failed to
realize that FP was using its own *Frm format class.

arcbbb accepted this revision.Dec 14 2021, 1:19 AM

LGTM

This revision is now accepted and ready to land.Dec 14 2021, 1:19 AM
asb accepted this revision.Dec 14 2021, 5:16 AM

Using AdjustInstrPostInstrSelection is a neat way to handle this I think. LGTM.

frasercrmck accepted this revision.Dec 14 2021, 6:01 AM

LGTM too. The only thing I'd be concerned about is something in LLVM dropping those implicit uses but we can probably fix those bugs if and when.

This revision was landed with ongoing or failed builds.Dec 14 2021, 10:18 AM
This revision was automatically updated to reflect the committed changes.