[llvm-dev] SDAG: Type legalization woes (context insensitivity)

Fri Mar 26 12:16:07 PDT 2021

There's a check for this in WidenVecRes_SETCC, but we're not reaching it
because we end up in a custom widening handler for SETCC.

  // The input and output types often differ here, and it could be that
while
  // we'd prefer to widen the result type, the input operands have been
split.
  // In this case, we also need to split the result of this node as well.
  if (getTypeAction(InVT) == TargetLowering::TypeSplitVector) {
    SDValue SplitVSetCC = SplitVecOp_VSETCC(N);
    SDValue Res = ModifyToType(SplitVSetCC, WidenVT);
    return Res;
  }

~Craig

On Fri, Mar 26, 2021 at 11:18 AM Krzysztof Parzyszek <kparzysz at quicinc.com>
wrote:

> I was able to come up with a different testcase that shows the same issue
> (where lack of context causes trouble):
>
>
>
> define <32 x i32> @fred(<32 x i32> %a0, <32 x i32> %a1) #0 {
>
> b0:
>
>   %v0 = bitcast <32 x i32> %a0 to <32 x float>
>
>   %v1 = bitcast <32 x i32> %a1 to <32 x float>
>
>   %v2 = fcmp ogt <32 x float> %v0, %v1
>
>   %v3 = select <32 x i1> %v2, <32 x float> zeroinitializer, <32 x float>
> %v0
>
>   %v4 = bitcast <32 x float> %v3 to <32 x i32>
>
>   ret <32 x i32> %v4
>
> }
>
>
>
> attributes #0 = { nounwind "target-cpu"="hexagonv66"
> "target-features"="+hvxv66,+hvx-length128b" }
>
>
>
> You can run this with “llc -march=hexagon < testcase.ll” and see it get
> stuck.
>
>
>
>
>
> Float vectors are illegal, and they need to be scalarized.  Integer
> vectors are legal, moreover we legalize short vectors by widening them to
> full vectors.  Boolean vectors (i.e. vNi1) are also widened accordingly if
> necessary, and herein lies the problem:
>
>
>
> First, this happens:
>
>
>
> Legalizing node: t8: v32i1 = setcc t5, t6, setogt:ch
>
> Analyzing result type: v32i1
>
> Legal result type
>
> Analyzing operand: t5: v32f32 = bitcast t2
>
> Split node operand: t8: v32i1 = setcc t5, t6, setogt:ch
>
>
>
> But then (v16i1 is not legal)
>
>
>
> Legalizing node: t449: v16i1 = setcc t26, t447, setogt:ch
>
> Analyzing result type: v16i1
>
> Widen node result 0: t449: v16i1 = setcc t26, t447, setogt:ch
>
>
>
> End we end up in an infinite loop, where we decide to widen the result of
> the short setcc, then to split the operands of the widened one, and so on…
>
>
>
> If, at the time of checking the v16i1, we knew that it came from a setcc
> that needs to be scalarized, we would not tell the legalizer to widen.  But
> we don’t know where the v16i1 came from, so we assume it came from an
> integer operation and we decide to widen based only on the type itself.  If
> we decided to scalarize v16i1, we may end up with the reverse problem,
> where the integer operands need to be widened, but the result wants to be
> scalarized…  We could decide to scalarize everything that is not legal, but
> that would defeat the purpose of widening altogether (and would be bad for
> performance).
>
>
>
>
>
> --
>
> Krzysztof Parzyszek  kparzysz at quicinc.com   AI tools development
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of *Krzysztof
> Parzyszek via llvm-dev
> *Sent:* Tuesday, March 23, 2021 1:09 PM
> *To:* Craig Topper <craig.topper at gmail.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] SDAG: Type legalization woes (context
> insensitivity)
>
>
>
> I had to go digging around a bit.  At the time we decided to go with
> something that worked, and now I thought I remembered enough about it…
>
>
>
> I cannot find the testcase, so I guess I’ll have to wait and see if the
> problem reappears.
>
>
>
>
>
> Sorry for the noise.
>
>
>
>
>
> --
>
> Krzysztof Parzyszek  kparzysz at quicinc.com   AI tools development
>
>
>
> *From:* Craig Topper <craig.topper at gmail.com>
> *Sent:* Monday, March 22, 2021 2:59 PM
> *To:* Krzysztof Parzyszek <kparzysz at quicinc.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] SDAG: Type legalization woes (context
> insensitivity)
>
>
>
> Where's the call to SetWidenedVector coming from? I would expect your case
> to go through WidenVecOp_SETCC which creates a wide setcc, but appends an
> extract_subvector before returning.
>
> ~Craig
>
>
>
> On Mon, Mar 22, 2021 at 12:52 PM Krzysztof Parzyszek <kparzysz at quicinc.com>
> wrote:
>
> We’re hitting the first one in this function (I’m trying to get a testcase
> that would reproduce this crash):
>
>
>
> void DAGTypeLegalizer::SetWidenedVector(SDValue Op, SDValue Result) {
>
>   assert(Result.getValueType() ==
>
>          TLI.getTypeToTransformTo(*DAG.getContext(), Op.getValueType()) &&
>
>          "Invalid type for widened vector");
>
>   AnalyzeNewValue(Result);
>
>
>
>   auto &OpIdEntry = WidenedVectors[getTableId(Op)];
>
>   assert((OpIdEntry == 0) && "Node already widened!");
>
>   OpIdEntry = getTableId(Result);
>
> }
>
>
>
>
>
> Yes, v64i1 is legal, but it fails the condition
> ‘getTypeToTransformTo(v32i1) == v64i1’.
>
>
>
>
>
> --
>
> Krzysztof Parzyszek  kparzysz at quicinc.com   AI tools development
>
>
>
> *From:* Craig Topper <craig.topper at gmail.com>
> *Sent:* Monday, March 22, 2021 1:55 PM
> *To:* Krzysztof Parzyszek <kparzysz at quicinc.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] SDAG: Type legalization woes (context
> insensitivity)
>
>
>
> Is v64i1 not a legal type for Hexagon? What assertion are you hitting?
>
>
> ~Craig
>
>
>
>
>
> On Mon, Mar 22, 2021 at 11:30 AM Krzysztof Parzyszek via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Summary: some types need more contextual information to properly legalize
> than what TLI queries give.
>
> Example: consider v32i1:
> 1.  v32i1 = compare v32i32, v32i32    ; all good
> 2.  v32i1 = compare v32i16, v32i16    ; problem: v32i16 not legal, but
> v32i1 is
>
>   The type v32i1 is legal because there it can be represented in a
> register and can be a result of a legal comparison of legally-typed
> vectors.  But it can also be a result of comparing illegally-typed vectors,
> which causes a problem.
>   The type legalization first looks at the value type, sees v32i1, takes
> note that it's legal.  Then, for the second case, it will attempt to
> legalize the operand types, but that action would also cause the result
> type to change (e.g. widen everything: v32i16->v64i16, v32i1->v64i1),
> hitting an assertion.
>
> I'm seeing this issue on Hexagon, but it seems to be fairly generic in its
> nature.  Has anyone else experienced this?
>
> I want to fix this somehow.  My current idea is to simply allow a known
> legal type to be replaced with another legal type, but there may be
> different approaches, like using SDValue in the TLI.whatAbouThisType family.
>
> Does anybody have concerns, thoughts?
>
> --
> Krzysztof Parzyszek  kparzysz at quicinc.com   AI tools development
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210326/1ad9cc60/attachment.html>