This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Add a combine for and(load , mask) -> zextload
ClosedPublic

Authored by kschwarz on Sep 7 2021, 5:31 AM.

Details

Summary

This only handles simple masks, not shifted masks, for now.

Diff Detail

Event Timeline

kschwarz created this revision.Sep 7 2021, 5:31 AM
kschwarz requested review of this revision.Sep 7 2021, 5:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 7 2021, 5:31 AM
Herald added a subscriber: wdng. · View Herald Transcript
paquette added inline comments.Sep 7 2021, 9:19 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
656

Pull this into a variable and reuse it in NewSize?

668

May want a comment explaining why we don't want powers of 2 under 8.

675

Reuse LoadReg?

683

Maybe use applyBuildFn instead of creating a new apply function?

arsenm added inline comments.Sep 7 2021, 5:19 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
675

Just save the type above?

kschwarz updated this revision to Diff 371278.Sep 8 2021, 1:51 AM
kschwarz marked 4 inline comments as done.

Address review comments

foad added inline comments.Sep 8 2021, 6:00 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
670

This looks like it will combine:

%1:_(s32) = G_CONSTANT i32 65535
%2:_(s32) = G_SEXTLOAD %0 :: (load (s8))
%3:_(s32) = G_AND %2, %1

into:

%2:_(s32) = G_ZEXTLOAD %0 :: (load (s8))

which would be wrong.

kschwarz updated this revision to Diff 371529.Sep 9 2021, 3:10 AM

Only apply combine if the mask does not exceed in-memory size

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
670

Good catch, thanks!

aemerson accepted this revision.Sep 14 2021, 9:22 PM
aemerson added inline comments.
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
637

We can use the BuildFnTy typedef here.

This revision is now accepted and ready to land.Sep 14 2021, 9:22 PM
This revision was automatically updated to reflect the committed changes.
kschwarz marked an inline comment as done.