[PATCH] D137274: AMDGPU/GlobalISel: Fix combine crash because LI is not set in prelegalizer

Petar Avramovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 08:40:58 PDT 2022


Petar.Avramovic updated this revision to Diff 472953.
Petar.Avramovic added a comment.

Correction:

> Change is caused by zextload being combined pre-legalizer

Change is caused by zextload NOT being combined pre-legalizer

s16 zextload is not legal and with LI availabe in prelegalizer combiner does not perform zextload combine.
Regression is caused by additional zext.

Using `if(!isPreLegalize())` as an old equivalent of `if(LI)` to avoid regression but I am not sure if it is correct behavior with the prelegalizer now also checking for legality. Overall I would prefer to use isPreLegalize() in this patch and fix LI uses in prelegalizer in another patch if needed.

If we don't make zextload in prelegalizer (and let legalizer deal with it) it becomes very difficult to combine, we are left with zext + lowered any_extending_load. It would require known bits analysis that would try to treat that anyextending load as zero or sign extending load (whichever is better) and perform combine and change type of load. This is not visible in the asm since we select any_extending load in the same way as zero_extending load.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137274/new/

https://reviews.llvm.org/D137274

Files:
  llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/select-to-fmin-fmax.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137274.472953.patch
Type: text/x-patch
Size: 8345 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221103/f5b4cc7b/attachment.bin>


More information about the llvm-commits mailing list