[PATCH] D71960: [Attributor] AAUndefinedBehavior: AAValueSimplify in memory accessing instructions.

[Hideto Ueno via Phabricator via llvm-commits]

uenoku added inline comments.


================
Comment at: llvm/test/Transforms/Attributor/ArgumentPromotion/2008-07-02-array-indexing.ll:12
-; CHECK-NEXT:    [[A_0:%.*]] = load i32, i32* null
-; CHECK-NEXT:    br i1 false, label [[T:%.*]], label [[F:%.*]]
 ; CHECK:       T:
----------------
baziotis wrote:
> uenoku wrote:
> > baziotis wrote:
> > > jdoerfert wrote:
> > > > We should also repair this test. pass a valid pointer into `callee`, e.g., an argument. 
> > > Ok,yes. Btw, the `nonnull` and `dereferenceable` attributes for `%A` are wrong right? I guess these are the kind of checks for violations you have proposed that AAUB could do.
> > > Btw, the nonnull and dereferenceable attributes for %A are wrong right? 
> > I think to mark nonnull and dereferenceable attributes for %A itself is not wrong. 
> > (but anyway %A will be deleted with D68765 or additional patches)
> > I mean **passing** null to this function is UB too. So `AAUB` can prove 
> > ```
> > %X = call i32 @callee(i1 false, i32* null) 
> > ```
> > is UB. 
> So, generally for attributes, I'm not sure when is the caller responsible for it to hold or the "marker" (or both).
> What I mean is that the way I see it, in this example, there are 2 cases (in the general case, replace "parameter" with "value that can be attributed"):
> 1) If the caller is responsible, then I can mark any attributes in the parameter (e.g. `nonnull`) and whoever calls this function is responsible for the attributes to hold (e.g. not pass a `null`). Otherwise, it is UB.
> 2) If the marker is responsible, then whoever marks a parameter (e.g. `nonnull`)  is responsible to validate that anyone who
> calls this function does not pass a `null` parameter.
> 
> The 1) makes more sense since 2) is practically impossible if the function is external. But 2) isn't really what the Attributor does?
> (e.g. the Attributor tries to be sure that an argument is `dereferenceable` to mark it as such).
> If the caller is responsible, then I can mark any attributes in the parameter (e.g. nonnull) and whoever calls this function is responsible for the attributes to hold (e.g. not pass a null). Otherwise, it is UB.
> If the marker is responsible, then whoever marks a parameter (e.g. nonnull) is responsible to validate that anyone who calls this function does not pass a null parameter.
> The 1) makes more sense since 2) is practically impossible if the function is external. But 2) isn't really what the Attributor does?
Yes, almost all the attributes are using only 2) but some (ex. nonnull, align, dereferenceable) are doing deduction in both ways.  In this case, `%A` is known as `nonnull` because `%A` is loaded at an entry point. 

If the function is an external function, Attributor gives up 2).  (please look at `checkForAllCallsites` implementation)


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

https://reviews.llvm.org/D71960