[llvm] [CodeGen] Improve `getLoadExtAction` and friends (PR #181104)

[Demetrius Kanios via llvm-commits]

QuantumSegfault wrote:

One thing I don't quite understand is the relationship between `getLoadExtAction` and `getOperationAction(ISD::LOAD...`

The default for `getLoadExtAction` for `NON_EXTLOAD` is `Legal`... so it returns Legal even in cases where such a load actually ISN'T... (e.g. i64 MemType into i64 register in AMDGPU?).

It's like there's two layers? A very coarse check of whether a particular ValTy can be loaded into, then the more detailed getLoadExtAction for information about the actual legality of specific memory ops (i.e. MemTy loading from and ext-type).

But it seems some DAGCombiners check getLoadExtAction without verifying with `getOperationAction`, so they can still generate illegal loads??? 

Anybody have any insight? 

I'm specifically looking at the `@test_smul24_i33` in the `CodeGen/AMDGPU/mul_int24.ll` test and it triggering `DAGCombiner::MatchLoadCombine`; around line 9900 in `DAGCombiner.cpp` is it calling isLoadExtLegal with MemTy i64, ValTy i64, NON_EXTLOAD during the `@test_smul24_i33`. It's "Legal" under `SI` (despite `isOperationLegal` for `ISD::LOAD` and an i64 MemTy being false), but not under `GFX9`??? It later gets transformed to a v2i32 instead of an i64...but why?


-----

I guess the main thing that matters is whether this change is enough, or a similar expansion of input needs to happen for what is currently just `getOperationAction(ISD::LOAD, ValVT)`.

These changes should suffice for my purposes in the WASM backend. We can also make the change more obvious/breaking by removing the tables and `setLoadExtAction` and requiring each target to implement the `getLoadAction` instead.

https://github.com/llvm/llvm-project/pull/181104