thr3ads.net - llvm dev - [llvm-dev] SDAG: Type legalization woes (context insensitivity) [Mar 2021]

If this information is useful, please help other people find it:
Share via:

Krzysztof Parzyszek via llvm-dev

2021-Mar-23 18:09 UTC

[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<mailto: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<mailto: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<mailto:kparzysz at
quicinc.com>   AI tools development

From: Craig Topper <craig.topper at gmail.com<mailto:craig.topper at
gmail.com>>
Sent: Monday, March 22, 2021 1:55 PM
To: Krzysztof Parzyszek <kparzysz at quicinc.com<mailto:kparzysz at
quicinc.com>>
Cc: llvm-dev <llvm-dev at lists.llvm.org<mailto: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<mailto: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<mailto:kparzysz at
quicinc.com>   AI tools development

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto: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/20210323/f54c0270/attachment-0001.html>

Krzysztof Parzyszek via llvm-dev

2021-Mar-26 18:18 UTC

head link

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

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<mailto: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<mailto:kparzysz at
quicinc.com>   AI tools development

From: Craig Topper <craig.topper at gmail.com<mailto:craig.topper at
gmail.com>>
Sent: Monday, March 22, 2021 2:59 PM
To: Krzysztof Parzyszek <kparzysz at quicinc.com<mailto:kparzysz at
quicinc.com>>
Cc: llvm-dev <llvm-dev at lists.llvm.org<mailto: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<mailto: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<mailto:kparzysz at
quicinc.com>   AI tools development

From: Craig Topper <craig.topper at gmail.com<mailto:craig.topper at
gmail.com>>
Sent: Monday, March 22, 2021 1:55 PM
To: Krzysztof Parzyszek <kparzysz at quicinc.com<mailto:kparzysz at
quicinc.com>>
Cc: llvm-dev <llvm-dev at lists.llvm.org<mailto: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<mailto: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<mailto:kparzysz at
quicinc.com>   AI tools development

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto: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/aa1bd7a9/attachment-0001.html>

llvm dev - Mar 2021 - SDAG: Type legalization woes (context insensitivity)

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

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