This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Don't ignore requested ext narrowing type
ClosedPublic

Authored by arsenm on Jan 13 2020, 5:40 AM.

Details

Summary

This was assuming the narrow target was the source type. Respect the
requested type when these don't match by using intermediate
merges. This avoids producing very wide, illegal shift expansions.

Diff Detail

Event Timeline

arsenm created this revision.Jan 13 2020, 5:40 AM
paquette added inline comments.Jan 13 2020, 10:59 AM
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
79

It would be nice to have the assert explain why OrigTy.isVector() && TargetTy.isVector() isn't handled.

249

Can we have a comment explaining the purpose of this function?

262–267

Can we change the bracing/logic here so that the else isn't the only thing with braces? I think it's fine to put braces around the if here.

Also a comment on the else/if cases would be nice.

310

Comment for what AllPadReg is?

311–313

Can we have a comment explaining what these loops are doing?

315–319

Can we avoid having the if only have braces here?

e.g.

if (Idx >= NumOrigSrc) {
  SubMerge[J] = PadReg;
  continue;
}

SubMerge[J] = VRegs[Idx];
AllMergePartsArePadding = false;

Also I'd like a comment explaining why Idx < NumOrigSrc sets AllMergePartsArePadding to false.

322–330

Comment for why we don't set AllPadReg on G_SEXT?

arsenm updated this revision to Diff 237785.Jan 13 2020, 2:34 PM

Address comments

arsenm marked 7 inline comments as done.Jan 13 2020, 2:35 PM
This revision is now accepted and ready to land.Jan 16 2020, 9:23 AM