In D66664#1643543, @efriedma wrote:

getPointerDereferenceableBytes returns some number of dereferenceable bytes. If CanBeNull is true, that result is modified: if the pointer value is null, the number of known dereferenceable bytes is actually zero.

I guess IR test should be affected already and it would be good to have the run time test that breaks with barriers.

We have uses of __SYNCWARP in the library, right? Can we use the function instead and maybe put it in the language dependent part?

Make sure to update the commit message as well.

thx for the explanation. LGTM.

Generally, this seems fine but I was hoping we could say what configuration and test file can be used to reproduce this error.

It seems inconsistent to declare Op0C initially and Op1C late given that both are need 2 times in almost the same fashion.

This is what I wanted but I think I need to see this in action as I'm not a template wizard. (Btw. if you want to clean up the mess I made, feel free!)

In D66461#1642252, @hfinkel wrote:

As I described, the compare will capture only if the other pointer was captured.

But the other pointer can be captured afterward, no?

In D66608#1642250, @reames wrote:

When I went to add the suggested size-0 type bailout, I realized the existing code was correct for this case. I added tests and a comment to show this.

Update tests and fix errors

In D64533#1641523, @reames wrote:

See https://reviews.llvm.org/D66608 for an alternate approach in InstCombine as suggested in review here.

In D66608#1641754, @nikic wrote:

As mentioned on the previous review, do we need to handle getelementptr inbounds {}, {}* %p, i32 %idx? That is, an inbounds GEP on a zero-sized type, which may be null for a non-zero index. It looks like this case is not handled in the existing code either though, so possibly this construction isn't legal (though I don't see anything to that effect in langref)?

In D66461#1641668, @efriedma wrote:

As I described, the compare will capture only if the other pointer was captured. So additionally to the checks in the patch ask the tracker if that is the case

You can't actually reconstruct the pointer in that case, but the bits are still significant; if two functions sort a list of pointers, they have to sort the same way. Whether that means the pointer is "captured" depends on the definition of "captured"; I can think of transforms that don't care, and transforms that do care.

The original code tried to do the modulo computation (as per comment and the looks of it) but the operands of DiffUnitsSCEV = SE->getMinusSCEV(DiffAlign, DiffSCEV) were swapped.
Swapping them should yield the same result as using URem but using URem is better so this is fine.

In D66461#1639801, @efriedma wrote:

Yes, you can't capture a pointer that's already captured.

! In D66461#1640565, @hfinkel wrote:
If you compare the same pointer (or derived from the same), you need to capture at least one from them "explicitly" to learn the bits.

Yes, but then we'd need some way to account for that. If pointer A is compared to pointer B, and then pointer B is unambiguously captured, then so has been pointer A (at least in the part of the control flow dominated by the true edge of the comparison).

LGTM

Use the call site as a key for the cache

In D66461#1639246, @hfinkel wrote:

I'm missing something here. If I have two pointers (e.g., returned from malloc), and I compare them so I know they're equal, then I've learned that the pointers are equal -- and, thus, if I know all of the bits of one pointer I now know all of the bits of the other pointer. As a result, I can still reconstruct it later using that information.

We'll need "nofree" for call site arguments ;)

Thx @efriedma, We'll do it without this change

In D66439#1638284, @efriedma wrote:

Is there some practical issue I'm missing?

In D53342#1637822, @xbolva00 wrote:

For some cases I would like to copy just attributes for argno 0 - but I dont know to do it :(

I mean something like this:

NewCI->setAttrubutes(0, CI->getAttributes(0))

Maybe you can help me?

Remove accidentally included code

Can you use ` (three backticks) to enclose the code regions for better formatting?

Fix typo and add baseline tests