This is an archive of the discontinued LLVM Phabricator instance.

[SLP] Factor out scalar type as a getter. NFC
Needs ReviewPublic

Authored by eopXD on May 27 2022, 7:48 AM.

Diff Detail

Event Timeline

eopXD created this revision.May 27 2022, 7:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2022, 7:48 AM
eopXD requested review of this revision.May 27 2022, 7:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2022, 7:48 AM
RKSimon added inline comments.May 27 2022, 9:42 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
221

(style)

if (auto*SI = dyn_cast<StoreInst>(V))
  return SI->getValueOperand()->getType();
if (auto *CI = dyn_cast<CmpInst>(V))
  return CI->getOperand(0)->getType();
if (auto *IE = dyn_cast<InsertElementInst>(V))
  return IE->getOperand(1)->getType();
return V->getType();
eopXD retitled this revision from [SLP][NFC] Factor out scalar type as a getter to [SLP] Factor out scalar type as a getter. NFC.May 27 2022, 3:15 PM
eopXD updated this revision to Diff 432654.May 27 2022, 3:18 PM

Update code

  • Address comment
eopXD marked an inline comment as done.May 27 2022, 3:18 PM

Is this really a NFC? Could some of the cases that just call getType() now call one of the dyn_cast variants?

Anyway, no objections from me - @ABataev any comments?

eopXD added a comment.May 30 2022, 1:59 PM

Thank you @RKSimon for raising this question.

Regarding the code that calls ->getType() and don't have dynamic casts...

  • Line 3494 can only be entered when instruction is a Instruction::Load.
  • Use of line 3614 can only be used when TE.Scalars consist of Load instructions.
  • Line 4909 is under case Instruction::Load
  • Line 11147 is under the reduction part of SLP, by the cases in RecurKind I think load / store / cmp instructions are not involved here.

With all this said, I am not sure if any existing code will cause this "none functional change intended" patch to fail.

ABataev added inline comments.May 31 2022, 3:47 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
7196

What if we gather cmps here?

10071

What if we vectorize list of cmps here?