This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add a VL output to vleff intrinsics.
ClosedPublic

Authored by craig.topper on Jan 7 2021, 9:24 PM.

Details

Summary

The fault-only-first-load instructions can reduce VL if an element
other than element 0 triggers a memory fault. This can be used to
vectorize loops with data dependent exit conditions like strcmp or
strlen.

This patch adds a VL output to these intrinsics so that the new
VL value can be captured by software. This will be expanded to
'csrr gpr, vl' after the vleff instruction during SelectionDAG.

By doing this with one intrinsic we are able to guarantee that the
csrr reads the VL value produced by the vleff instruction. Having
it as a separate intrinsic would make it impossible to guarantee
ordering without making every other vector intrinsic have side
effects.

The intrinsics are expanded during lowering into two ISD nodes
that are glued together. These ISD nodes will go
through isel separately, but should maintain the glue so that they
get emitted adjacently by InstrEmitter.

I've only ran the chain through the vleff instruction, allowing
the READ_VL to be deleted if it is unused.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 7 2021, 9:24 PM
craig.topper requested review of this revision.Jan 7 2021, 9:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2021, 9:24 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
craig.topper added inline comments.
llvm/test/CodeGen/RISCV/rvv/vleff-rv32.ll
4

I added the missing test cases for double to rv32.

-Rebase
-Add PseudoReadVL expansion. Remove string from Pseudo in .td.

This seems a very sensible approach.

Having the chain prevents it from being removed if it isn't used.

I don't think this too problematic (as you mention, using this instruction and not using the output vl may not be very useful). I'm curious, though: do you foresee any issue if it gets removed?

HsiangKai added inline comments.Jan 19 2021, 6:14 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
110

Why do we need IntrHasSideEffects?

Remove side effects from READ_VL portion. Allowing it to be deleted.

Add test cases for unused results.

craig.topper edited the summary of this revision. (Show Details)Jan 20 2021, 12:51 PM
craig.topper added inline comments.Jan 20 2021, 3:35 PM
llvm/include/llvm/IR/IntrinsicsRISCV.td
130

This should have IntrHasSideEffects

-Add tests for unused results on the masked intrinsic
-Make the masked intrinsic have "side effects"
-Use the default read/write memory property instead of IntrReadMem+IntrHasSideEffects which doesn't work correctly without tablegen changes.

Rebase after the argument register change

This revision is now accepted and ready to land.Jan 21 2021, 5:02 PM
This revision was landed with ongoing or failed builds.Jan 21 2021, 5:21 PM
This revision was automatically updated to reflect the committed changes.