[PATCH] D157826: [X86] Allow inlining callees missing VLX feature

[Kal Conley via Phabricator via llvm-commits]

kalcutter added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:6082
         // Do a precise compatibility check.
         if (!areTypesABICompatible(Caller, NestedCallee, Types))
           return false;
----------------
pengfei wrote:
> kalcutter wrote:
> > kalcutter wrote:
> > > nikic wrote:
> > > > I assume that this is the actually failing check? In that case, should the adjustment be in that function?
> > > I am not sure. That function is checking that the used types are ABI compatible, but what about available target instructions? Is the idea to first only check the types, then later during CodeGen the target does a more exhaustive check?
> > > 
> > > Fixing areTypesABICompatible() would be more general I guess. Do you have an idea how much work is involved properly fixing that function? I am not familiar with this code base at all. Do you have anything against applying this patch as an incremental improvement?
> > I guess the feature subset check covers all cases of available target instructions. So just making sure all the types are compatible, like you suggested, would be the proper fix.
> The function is designed to check if both caller and callee allow ZMM codegen or not when passing a 512-bit vector type. The logic to check ZMM codegen is:
> 1. has AVX512F but not AVX512VL
> 2. has AVX512F and `prefer-vector-width` >= 512
> 3. has AVX512F and `min-legal-vector-width` > 256
> 
> So has AVX512VL or not does affect the ABI when `prefer-vector-width` and `min-legal-vector-width` are set 256 so less, though it shouldn't happen since we have guarantee they are set properly in the FE.
I am a bit confused. A function with both AVX512F and AVX512VL features obviously allows ZMM codegen in the function body. Why should enabling AVX512VL change how 512-bit vector types are passed? Even if that were the case for an opaque function, why should that disabling inlining? Also I don't see how (1)-(3) should ever affect`always_inline`.


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

https://reviews.llvm.org/D157826