Page MenuHomePhabricator

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

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

Details

Summary

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

Diff Detail

Event Timeline

kschwarz created this revision.Tue, Sep 7, 5:31 AM
kschwarz requested review of this revision.Tue, Sep 7, 5:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Sep 7, 5:31 AM
Herald added a subscriber: wdng. · View Herald Transcript
paquette added inline comments.Tue, Sep 7, 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.Tue, Sep 7, 5:19 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
675

Just save the type above?

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

Address review comments

foad added inline comments.Wed, Sep 8, 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.Thu, Sep 9, 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.Tue, Sep 14, 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.Tue, Sep 14, 9:22 PM
This revision was automatically updated to reflect the committed changes.
kschwarz marked an inline comment as done.