[PATCH] D74700: [IR] Remove temporary const operator created in Value::getPointerAlignment()

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 18 01:19:12 PST 2020


lebedev.ri added a comment.

In D74700#1879865 <https://reviews.llvm.org/D74700#1879865>, @jdoerfert wrote:

> In D74700#1879625 <https://reviews.llvm.org/D74700#1879625>, @efriedma wrote:
>
> > > If we can cleanup instances after some unexpected changes why not to do so?
> >
> > The big problem here is that getPointerAlignment could be used in places where not all uses of a constant are actual "Use" instances.  Then you have a really subtle use-after-free.  (This could happen in places with maps on the side, like frontends or complex transforms).
> >
> > If you really want to avoid creating extra constants, probably the right strategy is to avoid calling getPtrToInt in the first place.  There aren't that many ways to construct a pointer that getPtrToInt can fold to a ConstantInt, anyway. (Off the top of my head, it would have to be null, an inttoptr, or a gep of one of those.)
>
>
> I was hoping that would be a viable solution. Maybe matching `null` explicitly and otherwise just dealing with the constant that we have. IIRC, @lebedev.ri mentioned that would not work well but I let him explain.


I mean, sure, we can avoid using `getPtrToInt` and hardcode what it does here, but still, why?
Either the current state breaks guarantees and we should fix it,
or it does not and this roughly falls under the "c++ api is unstable" and whatever code
was relying on the unwritten guarantee should be relaxed not to rely on it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74700/new/

https://reviews.llvm.org/D74700





More information about the llvm-commits mailing list