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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

I don't believe the question

In D73131#1865791 <https://reviews.llvm.org/D73131#1865791>, @lebedev.ri wrote:

> In D73131#1861998 <https://reviews.llvm.org/D73131#1861998>, @jdoerfert wrote:
>
> > In D73131#1861978 <https://reviews.llvm.org/D73131#1861978>, @lebedev.ri wrote:
> >
> > > In D73131#1861781 <https://reviews.llvm.org/D73131#1861781>, @jdoerfert wrote:
> > >
> > > > In D73131#1861624 <https://reviews.llvm.org/D73131#1861624>, @mlychkov wrote:
> > > >
> > > > > In D73131#1861595 <https://reviews.llvm.org/D73131#1861595>, @mlychkov wrote:
> > > > >
> > > > > > ...
> > > > > >  Is this alignment accounting intended to be used for getelementptr instructions too?
> > > > > >  If yes then may we cleanup extra user after we get TrailingZeros value?
> > > > >
> > > > >
> > > > > Or is there a way to perform this computation without const_cast?
> > > >
> > > >
> > > > The problem is `ConstantExpr::getPtrToInt`. Do we striclty need that?
> > >
> > >
> > > That is the workhorse here, we can't get around using it.
> > >
> > > Without seeing the code where this is causing trouble i'm somewhat unsure
> > >  on how this is a problem, or how to test that the fix would actually fix it.
> > >  Perhaps, after computing `TrailingZeros`, we need to explicitly DCE `CstInt`?
> >
> >
> > That'll probably work as well. If `CstInt` has 0 users it can be removed.
>
>
> Do we have an established precedent on this kind of issues?
>  I don't recall ever seeing such a problem before,
>  nor do i recall seeing manual cleaning up of tmp constantexprs.
>  I'm mainly wondering whether we actually have such guarantees that this violated (should not create new users)?




In D73131#1865811 <https://reviews.llvm.org/D73131#1865811>, @nikic wrote:

> Sounds pretty odd to me too. From a quick grep, there's only a handful of `destroyConstant()` uses outside GlobalOpt, and those tend to deal with `BlockAddress` only (which is somewhat particular about uses).


was answered. Do we have this guarantee in the first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74700