This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Disallow scale for scatter/gather
ClosedPublic

Authored by reames on Sep 21 2022, 1:18 PM.

Details

Summary

RISCV doesn't actually support a scaled form of indexed load and store. We previously handled this by forming the scaled SDNode, and then doing custom legalization during lowering. This patch instead adds a callback via TLI to prevent formation entirely.

This has two effects:

  • First, the GEP gets expanded (and used). Instead of the shift being created with an SDLoc of the memory operation, it has the SDLoc of the GEP instruction. This avoids the scheduler perturbing IR order when there's no reason to.
  • Second, we fix what appears to be a bug in index calculation with RV32. The rules for GEPs require index calculation be done in particular bitwidth, and it appears the custom legalization code got this wrong for the case where index type exceeds pointer width. (Or at least, I trust the generic GEP lowering to be correct a lot more.)

The DAGCombiner change to handle VPScatter/VPGather is technically separate, but is required to prevent a regression on those intrinsics.

Diff Detail

Build Status
Buildable 188193

Event Timeline

reames created this revision.Sep 21 2022, 1:18 PM
reames requested review of this revision.Sep 21 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2022, 1:18 PM
craig.topper added inline comments.Sep 21 2022, 5:23 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4441

Since the target is required to support 1 should be skip the call for 1?

llvm/lib/Target/RISCV/RISCVISelLowering.h
598

80 columns

reames updated this revision to Diff 462199.Sep 22 2022, 9:16 AM

Address review comments

This revision is now accepted and ready to land.Sep 22 2022, 2:27 PM
This revision was automatically updated to reflect the committed changes.