[PATCH] D157603: [RISCV] Truncate constants to EleSize when combine store of BUILD_VECTOR

[Wang Pengcheng via Phabricator via llvm-commits]

wangpc added a comment.

The root cause of this assertion is:

- The stacktrace is:

  llvm::SelectionDAG::getConstant(llvm::ConstantInt const&, llvm::SDLoc const&, llvm::EVT, bool, bool) (llvm\lib\CodeGen\SelectionDAG\SelectionDAG.cpp:1571)
  llvm::SelectionDAG::getConstant(llvm::APInt const&, llvm::SDLoc const&, llvm::EVT, bool, bool) (llvm\lib\CodeGen\SelectionDAG\SelectionDAG.cpp:1555)
  llvm::TargetLowering::SimplifyDemandedBits(llvm::SDValue, llvm::APInt const&, llvm::APInt const&, llvm::KnownBits&, llvm::TargetLowering::TargetLoweringOpt&, unsigned int, bool) const (llvm\lib\CodeGen\SelectionDAG\TargetLowering.cpp:2778)
  llvm::TargetLowering::SimplifyDemandedBits(llvm::SDValue, llvm::APInt const&, llvm::KnownBits&, llvm::TargetLowering::TargetLoweringOpt&, unsigned int, bool) const (llvm\lib\CodeGen\SelectionDAG\TargetLowering.cpp:646)
  (anonymous namespace)::DAGCombiner::SimplifyDemandedBits(llvm::SDValue, llvm::APInt const&) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:335)
  (anonymous namespace)::DAGCombiner::SimplifyDemandedBits(llvm::SDValue) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:329)
  (anonymous namespace)::DAGCombiner::visitTRUNCATE(llvm::SDNode*) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:14588)
  (anonymous namespace)::DAGCombiner::visit(llvm::SDNode*) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:1987)
  (anonymous namespace)::DAGCombiner::combine(llvm::SDNode*) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:2064)
  (anonymous namespace)::DAGCombiner::Run(llvm::CombineLevel) (llvm\lib\CodeGen\SelectionDAG\DAGCombiner.cpp:1856)

- The code should be responsible for the assertion:

`llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp`

  SDValue SelectionDAG::getConstant(const ConstantInt &Val, const SDLoc &DL,
                                    EVT VT, bool isT, bool isO) {
    //...
    // In some cases the vector type is legal but the element type is illegal and
    // needs to be promoted, for example v8i8 on ARM.  In this case, promote the
    // inserted value (the type does not need to match the vector element type).
    // Any extra bits introduced will be truncated away.
    if (VT.isVector() && TLI->getTypeAction(*getContext(), EltVT) ==
                             TargetLowering::TypePromoteInteger) {
      EltVT = TLI->getTypeToTransformTo(*getContext(), EltVT);
      APInt NewVal = Elt->getValue().zextOrTrunc(EltVT.getSizeInBits());
      Elt = ConstantInt::get(*getContext(), NewVal);
    }
    // ...
  }

Somehow we need to create a `v64i1` constant with all elements is `0` when combine `TRUNCATE`. And element type `i1` is illegal, so we get `v64i64` actually.

- DAG Combine log:

  Combining: t8: v64i1 = truncate t7
  Creating new node: t20: v64i1 = BUILD_VECTOR Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>
  
  Replacing.2 t8: v64i1 = truncate t7
  
  With: t20: v64i1 = BUILD_VECTOR Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>, Constant:i64<0>

For the test, the `insertelement` is a nop actually. If we change it to non-nop(we will actually insert a value), the assertion won't be triggered (because the different code path of DAGCombine is token):

  define void @assertion(ptr %p) {
    %v = insertelement <64 x i64> zeroinitializer, i64 0, i32 0
    %trunc = trunc <64 x i64> %v to <64 x i1>
    %p1 = getelementptr i8, ptr %p, i32 0
    %p2 = getelementptr i8, ptr %p, i32 8
    store <64 x i1> %trunc, ptr %p1
    store <8 x i8> zeroinitializer, ptr %p2
    ret void
  }
  
  define void @no_assertion(ptr %p) {
    %v = insertelement <64 x i64> zeroinitializer, i64 1, i32 0
    %trunc = trunc <64 x i64> %v to <64 x i1>
    %p1 = getelementptr i8, ptr %p, i32 0
    %p2 = getelementptr i8, ptr %p, i32 8
    store <64 x i1> %trunc, ptr %p1
    store <8 x i8> zeroinitializer, ptr %p2
    ret void
  }

So I think this kind of IR sequences won't be common in real code.
My fix is truncating all elemments to element size. I'm not so confident that this is the right place to fix, I will wait a few days before landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157603