Page MenuHomePhabricator

GlobalISel: Combine `op undef, x` to 0
ClosedPublic

Authored by volkan on Aug 26 2020, 4:32 AM.

Details

Summary

Add a combine to transform op undef, x to 0. Support only G_SHL for now.

Diff Detail

Event Timeline

volkan created this revision.Aug 26 2020, 4:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2020, 4:32 AM
volkan requested review of this revision.Aug 26 2020, 4:32 AM
volkan set the repository for this revision to rG LLVM Github Monorepo.Aug 26 2020, 4:32 AM
arsenm added inline comments.Aug 26 2020, 6:23 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1858

The isReg check should be unnecessary

llvm/test/CodeGen/AArch64/GlobalISel/combine-shl.mir
16

Also test a vector?

volkan added inline comments.Aug 31 2020, 10:18 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
1858

MO.getReg() may cause a crash without that check. This shouldn't a problem for this case, but I'd like to keep it as this function might be used by other combines too.

volkan updated this revision to Diff 288980.Aug 31 2020, 10:19 AM
  • Added a test case with vector type.
volkan marked an inline comment as done.Aug 31 2020, 10:20 AM
volkan updated this revision to Diff 288985.Aug 31 2020, 10:29 AM
  • Fixed liveins.
paquette added inline comments.Sep 1 2020, 9:43 AM
llvm/test/CodeGen/AArch64/GlobalISel/combine-shl.mir
4

A testcase where the G_IMPLICIT_DEF has more than one use would be good?

volkan added inline comments.Sep 3 2020, 9:06 AM
llvm/test/CodeGen/AArch64/GlobalISel/combine-shl.mir
4

Another user wouldn't have any impact on this case. Could you explain why that would be good?

paquette accepted this revision.Sep 4 2020, 1:49 PM

LGTM now that I realized I just misread the code here

llvm/test/CodeGen/AArch64/GlobalISel/combine-shl.mir
4

Oh, I misread the code. I thought you were replacing the G_IMPLICIT_DEF for some reason when I first read this. :P

This revision is now accepted and ready to land.Sep 4 2020, 1:49 PM