[PATCH] D103458: [SLP]Improve gathering of scalar elements.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D103458#2833713 <https://reviews.llvm.org/D103458#2833713>, @rupprecht wrote:

> We're seeing a test failure (and true miscompile AFAICT) in the CVC4 library that bisects to this patch. I don't have a nice reduction, but I can describe the issue we see.
>
> In `Constraint::externalExplain(AssertionOrder order)`, we construct a `NodeBuilder` [1]:
>
>   NodeBuilder<> nb(kind::AND);
>
> That constructor expands to this [2]:
>
>   inline NodeBuilder(Kind k) :
>     d_nv(&d_inlineNv),
>     d_nm(NodeManager::currentNM()),
>     d_nvMaxChildren(nchild_thresh) {
>     Assert(k != kind::NULL_EXPR && k != kind::UNDEFINED_KIND)
>         << "illegal Node-building kind";
>   
>     d_inlineNv.d_id = 1; // have a kind already
>     d_inlineNv.d_rc = 0;
>     d_inlineNv.d_kind = expr::NodeValue::kindToDKind(k);
>     d_inlineNv.d_nchildren = 0;
>   }
>
> `d_inlineNv` is a local class data member, and `d_nv` by default is just a pointer to that data member (but can be reassigned to point to something heap allocated)
>
> The fields of `d_inlineNv` should be zero except for `d_id` which is 1, and `d_kind` which is 21 (corresponding to `kind::AND`). However after this commit, the struct is initialized with poison. The IR diff we see is this:
>
>   define dso_local void @_ZNK4CVC46theory5arith10Constraint15externalExplainEm(%"class.CVC4::NodeTemplate"* noalias sret(%"class.CVC4::NodeTemplate") align 8 %agg.result, %"class.CVC4::theory::arith::Constraint"* nonnull align 8 dereferenceable(144) %this, i64 %order) local_unnamed_addr #15 align 32 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
>   ...
>   -  %40 = phi <2 x i64> [ %phi.bo220, %37 ], [ <i64 1, i64 21>, %if.else69 ]
>   +  %40 = phi <2 x i64> [ %phi.bo220, %37 ], [ poison, %if.else69 ]
>   ...
>     %42 = bitcast %"class.CVC4::NodeBuilder.623"* %nb to <2 x i64>*
>     store <2 x i64> %40, <2 x i64>* %42, align 16
>
> (`%phi.bo220` is a path never taken AFAICT)
>
> [1] https://github.com/CVC4/CVC4-archived/blob/40eac7f0529176bcc8464d6c4c8804fbde628c2b/src/theory/arith/constraint.cpp#L1563
> [2] https://github.com/CVC4/CVC4-archived/blob/40eac7f0529176bcc8464d6c4c8804fbde628c2b/src/expr/node_builder.h#L377

Hi, really need a reproducer and a command line. Otherwise, it is hard to understand why there is a miscompilation. This is rather strange that this patch could cause such miscompilation, I would suppose that a https://github.com/llvm/llvm-project/commit/f126e8ec2873ceedde53d2ccee1a66a83620e9a6#diff-e3b933e8c46156c0eaf7cbb67866b712f69a8484bc941d10acec9d4d17b9f061 could cause this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103458