[PATCH] D80562: AMDGPU/GlobalISel: Start rewriting load/store legality rules

[Matt Arsenault via Phabricator via llvm-commits]

arsenm created this revision.
arsenm added reviewers: foad, nhaehnle, kerbowa.
Herald added subscribers: hiraditya, t-tye, tpr, dstuttard, rovka, yaxunl, wdng, jvesely, kzhuravl.
Herald added a project: LLVM.

The current set is an incomprehensible mess riddled with ordering
hacks for various limitations in the legalizer at the time of writing,
many of which have been fixed. This takes a very small step in
correcting this.

      

The core first change is to start checking for fully legal cases
first, rather than trying to figure out all of the actions that could
need to be performed. It's recommended to check the legal cases first
for faster legality checks in the common case. This still has a table
listing some common cases, but it needs measuring whether this really
helps or not.

      

More significantly, stop trying to allow any arbitrary type with a
legal bitwidth as a legal memory type, and start using the bitcast
legalize action for them. Allowing loads of these weird vector types
produced new burdens we don't need for handling all of the
legalization artifacts. Unlike the SelectionDAG handling, this is
still not casting 64 or 16-bit element vectors to 32-bit
vectors. These cases should still be handled by increasing/decreasing
the number of 16-bit elements. This is primarily to fix 8-bit element
vectors.

      

Another change is to stop trying to handle the load-widening based on
a higher alignment. We should still do this, but the way it was
handled wasn't really correct. We really need to modify the MMO's size
at the same time, and not just increase the result type. The
LegalizerHelper does not do this, and I think this would really
require a separate WidenMemory action (or to add a memory action
payload to the LegalizeMutation). These will now fail to legalize.

The structure of the legalizer rules makes writing concise rules here
difficult. It would be easier if the same function could answer the
query the query, and report the action to perform at the same
time. Instead these two are split into distinct predicate and action
functions. This is mostly tolerable for other cases, but the
load/store rules get pretty complicated so it's difficult to keep two
versions of these functions in sync.


https://reviews.llvm.org/D80562

Files:
  llvm/include/llvm/CodeGen/GlobalISel/LegalizerInfo.h
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-constant.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-flat.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-global.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-local.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-load-private.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-phi.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-sextload-global.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-store.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-zextload-global.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80562.266224.patch
Type: text/x-patch
Size: 373631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200526/928ceab0/attachment-0001.bin>