This is an archive of the discontinued LLVM Phabricator instance.

Constant fold launder of null and undef
ClosedPublic

Authored by Prazek on Apr 24 2017, 2:34 AM.

Event Timeline

Prazek created this revision.Apr 24 2017, 2:34 AM
davide added a subscriber: davide.Apr 24 2017, 7:17 AM

I guess the patch is implemented correctly, but, can you please provide an example of clang emitting such construct?

I guess the patch is implemented correctly, but, can you please provide an example of clang emitting such construct?

Check out https://reviews.llvm.org/D32378
Clang will add barriers for some of pointer casts and pointer compares, so it is important to propagate null.
Propagating undef is only to make sure barriers don't limit optimizations like DCE etc.

This is fine but please hold committing it until the other half goes in.

test/Transforms/GVN/invariant.group.ll
488–490 ↗(On Diff #96358)

CHECK-NEXT:

493–495 ↗(On Diff #96358)

CHECK-NEXT:

davide accepted this revision.Apr 24 2017, 7:57 AM
This revision is now accepted and ready to land.Apr 24 2017, 7:57 AM
davide requested changes to this revision.Apr 24 2017, 7:58 AM
This revision now requires changes to proceed.Apr 24 2017, 7:58 AM
Prazek updated this revision to Diff 96400.Apr 24 2017, 8:23 AM
Prazek edited edge metadata.
Prazek marked 2 inline comments as done.
  • check-next
majnemer added inline comments.Apr 24 2017, 8:26 AM
lib/Analysis/InstructionSimplify.cpp
4432–4438 ↗(On Diff #96400)

This probably belongs in the constant folder.

Prazek added inline comments.Apr 24 2017, 8:36 AM
lib/Analysis/InstructionSimplify.cpp
4432–4438 ↗(On Diff #96400)

Do you mean folding barrier(const) -> const?

majnemer added inline comments.Apr 24 2017, 8:53 AM
lib/Analysis/InstructionSimplify.cpp
4432–4438 ↗(On Diff #96400)

Not for all constants, just undef and null in address space zero.

Prazek added inline comments.Apr 24 2017, 9:08 AM
lib/Analysis/InstructionSimplify.cpp
4432–4438 ↗(On Diff #96400)

I don't understand what you mean. Do you mean to move this code somewhere else?

Yes, move to ConstantFold*

Prazek updated this revision to Diff 96437.Apr 24 2017, 11:57 AM
Prazek marked 4 inline comments as done.

moved to constant folder

Thanks, I didn't know ConstantFolder exists

Prazek retitled this revision from Simplify barriers of null and undef to Constant fold barriers of null and undef.Apr 24 2017, 11:58 AM
majnemer added inline comments.Apr 24 2017, 1:38 PM
lib/Analysis/ConstantFolding.cpp
1779–1783

Isn't this only valid if the address space is zero?

Prazek added inline comments.Apr 24 2017, 2:25 PM
lib/Analysis/ConstantFolding.cpp
1779–1783

True, but can you pass pointer from another addrspace to invariant.group.barrier?

When I try

%X = addrspacecast i8* null to i8 addrspace(1)*
%b2 = call i8* @llvm.invariant.group.barrier(i8* %X)

I get

error: '%X' defined with type 'i8 addrspace(1)*'

I could add this line:

cast<PointerType>(Operands[0]->getType())->getAddressSpace() == 0

but I won't be able to test it in any way if I am not wrong.

davide added inline comments.May 8 2017, 12:03 PM
lib/Analysis/ConstantFolding.cpp
1779–1783

Not positive about this, but does it work if you do:

%b2 = call i8* @llvm.invariant.group.barrier(i8* addrspace(4)* @global0)

where

@global0 = addrspace(4) constant i8* null
Prazek added inline comments.May 20 2017, 2:48 AM
lib/Analysis/ConstantFolding.cpp
1779–1783

Thanks for help Davide, but it still doesn't work for me. I get the following error:

error: '@llvm.invariant.group.barrier' defined with type 'i8* (i8*)*'
  %b = call i8* @llvm.invariant.group.barrier(i8* addrspace(4)* @global0)
                ^

the code looks like this

@global0 = addrspace(4) constant i8* null

define i8* @differentAddrspace() {
  %b = call i8* @llvm.invariant.group.barrier(i8* addrspace(4)* @global0)
  ret i8* %b
}

declare i8* @llvm.invariant.group.barrier(i8*)
Prazek updated this revision to Diff 145366.May 5 2018, 4:57 AM

Rebasing

amharc added inline comments.May 5 2018, 5:03 AM
llvm/test/Transforms/InstCombine/invariant.group.ll
4 ↗(On Diff #145366)

nit: simplify (in other places too)

Prazek updated this revision to Diff 145367.May 5 2018, 5:13 AM
Prazek marked an inline comment as done.

Fixed typos

xbolva00 accepted this revision.May 5 2018, 10:15 AM
Prazek retitled this revision from Constant fold barriers of null and undef to Constant fold launder of null and undef.May 16 2018, 1:06 PM
kuhar accepted this revision.May 16 2018, 1:11 PM

LGTM

This revision was not accepted when it landed; it landed in state Needs Review.May 18 2018, 4:56 PM
This revision was automatically updated to reflect the committed changes.