Page MenuHomePhabricator

[CodeGen] Modify the refineIndexType(...)'s code to fix a bug in D90942.
ClosedPublic

Authored by yubing on Dec 3 2020, 12:18 AM.

Details

Summary

In previous code, when refineIndexType(...) is called and Index is undef, Index.getOperand(0) will raise a assertion fail.

Diff Detail

Event Timeline

yubing created this revision.Dec 3 2020, 12:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2020, 12:18 AM
yubing requested review of this revision.Dec 3 2020, 12:18 AM
yubing updated this revision to Diff 309174.Dec 3 2020, 12:29 AM

Delete some useles code

Thanks for finding and fixing this!

Can the test be simplified by using <4 x float> instead of <48 x float>?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9416

Probably better to explicitly check if Index is not a zero/sign extend, instead of checking whether it has any operands.

yubing updated this revision to Diff 309246.Dec 3 2020, 6:52 AM

Modified some code according to comments

yubing added a comment.Dec 3 2020, 6:53 AM

Thanks for finding and fixing this!

Can the test be simplified by using <4 x float> instead of <48 x float>?

Hi, thanks for comments. I can't change into <4 x float>.
I could change to <24 x float> because The smallest type to reproduce this bug is <24 x float>.

In the case of "<24 x float>", the scattered index is represented by a buildvector of<24 x i64> .
During LegalizeType procedure, The scattered index, aka, a buildvector of<24 x i64>will be widened into buildvector (<32 x i64>) whose last 8 elements are undef.
Then, <32 x i64> is split into two buildvectors(16 x i64) , one of which carries 8 undef.
Then, <16 x i64> is split into two buildvectors(8 x i64), one of which carries 8 undef, which means this buildvector will become a v8i64 undef(That's the masked.scatter's index which cause assertion fail).

Any thoughts?

yubing marked an inline comment as done.Dec 3 2020, 6:53 AM
pengfei added inline comments.Dec 3 2020, 7:01 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9421

How about just use Index = Index.getOperand(0)

yubing updated this revision to Diff 309418.Dec 3 2020, 5:42 PM

Modified some code according Pengfei's comments

yubing marked an inline comment as done.Dec 3 2020, 5:43 PM
yubing retitled this revision from [CodeGen] Check if Index's OperandNum == 0 when refineIndexType(...) is called This is a bugfix for D90942 to [CodeGen] Modify the refineIndexType(...)'s code to fix a bug in D90942..Dec 3 2020, 6:54 PM
yubing edited the summary of this revision. (Show Details)
pengfei accepted this revision.Dec 3 2020, 10:22 PM

LGTM.

This revision is now accepted and ready to land.Dec 3 2020, 10:22 PM