Page MenuHomePhabricator

[ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility
Needs ReviewPublic

Authored by skatkov on Aug 10 2018, 12:31 AM.

Details

Summary

GEP and BitCast instructions may produce a pointer of vectors from
underlying object. So it is valid for GetUnderlyingObject utility to
accept vector of pointers and detect underlying object which might be
instruction of vector or non-vector type.

Diff Detail

Event Timeline

skatkov created this revision.Aug 10 2018, 12:31 AM
skatkov retitled this revision from [ValueTracking] Accept vector of pointer in GetUnderlyingObject utility to [ValueTracking] Accept vectors of pointer in GetUnderlyingObject utility.
arsenm requested changes to this revision.Aug 10 2018, 1:12 AM
arsenm added a subscriber: arsenm.

Needs tests. I'm also not sure making this work for vectors necessarily makes sense

This revision now requires changes to proceed.Aug 10 2018, 1:12 AM

Thank you for your comment. Unfortunately till now I failed to create a test showing the benefit from this change in a pass (I tried). I can make one more try.
The problem I'm trying to solve is the following: in downstream pass I have a BitCast instruction which is actually originated from allocation but has a vector type gotten using gep + bitcast from allocation.
I'd like to know the fact that it is an underlying allocation but now utility function just ignore instructions with vector of pointers.

There are ValueTracking unittests already you could fall back on putting a test there, but I think a pass using this would be better

skatkov updated this revision to Diff 160083.Aug 10 2018, 3:21 AM

Thank you! I've added two unit tests.

ping. Can I do anything to proceed with this change?

arsenm added inline comments.Aug 16 2018, 11:22 PM
unittests/Analysis/ValueTrackingTest.cpp
318–322

It looks like a bug this IR is accepted by the verifier at all. This is a bitcast between different sized objects

arsenm added inline comments.Aug 16 2018, 11:25 PM
unittests/Analysis/ValueTrackingTest.cpp
318–322

I think fixing this is a prerequisite for making any changes like this to avoid issues

skatkov added inline comments.Aug 16 2018, 11:26 PM
unittests/Analysis/ValueTrackingTest.cpp
318–322

what if the size of the pointer is 32 bit?

skatkov added inline comments.Aug 16 2018, 11:41 PM
unittests/Analysis/ValueTrackingTest.cpp
318–322

It seems I'm not right here...

arsenm added inline comments.Aug 16 2018, 11:50 PM
unittests/Analysis/ValueTrackingTest.cpp
318–322

This is a cast between a 64-bit pointer and 2 64-bit pointers

skatkov updated this revision to Diff 161181.Aug 17 2018, 1:59 AM

Test is updated to be valid.

Verifier fix landed, can we proceed with this change?