[PATCH] Check for all known bits on ret in InstCombine

[Hal Finkel]

----- Original Message -----
> From: "Chandler Carruth" <chandlerc at google.com>
> To: "Philip Reames" <listmail at philipreames.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, reviews+D4567+public+8b075aecf68909a0 at reviews.llvm.org, "Commit Messages and
> Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Tuesday, July 22, 2014 1:40:05 PM
> Subject: Re: [PATCH] Check for all known bits on ret in InstCombine
> 
> 
> 
> 
> 
> On Tue, Jul 22, 2014 at 11:24 AM, Philip Reames <
> listmail at philipreames.com > wrote:
> 
> 
> 
> On 07/22/2014 10:25 AM, Hal Finkel wrote:
> 
> 
> 
> 
> 
> 2) When do we remove a llvm.assume? In your test case, you had an
> assume which (after the transform) retained a condition about an
> otherwise used value (%a). This seems like an obvious case to remove
> since we've gotten all the benefit we can from it. Are there other
> cases like this we should implement?
> No, I disagree.
> This is exactly the discussion we need to be having then. :)
> 
> 
> 
> We don't want to remove the invariants until after inlining is
> complete. And for alignment guarantees, for example, we don't want
> to remove them until after vectorization and unrolling. Currently,
> they're removed on entry to CodeGen, and it is not clear to me that
> we need or want to do anything else.
> I don't agree with this. I see several cases where removing them
> early seems like a good idea. Consider:
> - Once we recognize an assumption and canonicalize it as an
> attribute, we should immediately remove the assumption. The
> assumption conveys no additional information and only limits
> optimization. All of the semantic information is available in the
> attribute. If your concern is that we loose this information on
> inlining, then maybe we need to address *that*.
> - Once the values which contribute to the assumption become otherwise
> dead, what does information about those values tell us? It seems
> like there's no profit to be had by keeping it. (Thinking about it,
> my original question/proposal was wrong. "%a" was a parameter. There
> might be later uses of the value after inlining. For discussion,
> let's pretend "%a" had been the result of a pure function call or
> something.)
> I think these are the exceptions rather than the rules?
> 
> 
> Essentially, I think that "promoting" an assumption to an attribute
> clearly supplants the need for the assumption intrinsic.

Agreed, and we should do this whenever possible (align, nonnull, etc.) -- the attribute is the more efficient representation.

> And I think
> that we should clearly DCE *completely dead* sub-graphs that are
> being kept alive solely due to the assumption.

I suppose the real question is: what does completely dead mean? If I have:

int a;
void foo() {
  __builtin_assume(a == 5);
}

is this completely dead? I would say no so long as foo() could be inlined anywhere.

Thanks again,
Hal

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory