[PATCH] D27518: Moving isComplex decision to TTI

[Elena Demikhovsky via Phabricator via llvm-commits]

delena added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:606
+  /// of address-computation cost 
+  struct AddressAccessInfo {
+    bool  isStrided;        /// True in case the access is strided (AddRec).
----------------
mkuper wrote:
> magabari wrote:
> > mkuper wrote:
> > > magabari wrote:
> > > > mkuper wrote:
> > > > > hfinkel wrote:
> > > > > > mkuper wrote:
> > > > > > > delena wrote:
> > > > > > > > mkuper wrote:
> > > > > > > > > delena wrote:
> > > > > > > > > > Why do you need a struct? "Value *" will contain everything.
> > > > > > > > > > 
> > > > > > > > > > Value *Stride;
> > > > > > > > > > 
> > > > > > > > > > if (Stride == nullptr) - no stride,
> > > > > > > > > > dyn_cast<ConstantInt>(Stride) - answers the question isConstant()
> > > > > > > > > >  
> > > > > > > > > IIUC, we don't necessarily have a Value* in hand, this originates from a SCEV, right?
> > > > > > > > yes. We can use const SCEV*. 
> > > > > > > I'm not sure passing a SCEV* to TTI makes sense, in terms of layering.
> > > > > > I don't see why that should be a problem. The backends already depend on SCEV (and the rest of analysis) because of IR-level passes. TTI should definitely use SCEV where appropriate.
> > > > > Never mind me, then. SCEV it is.
> > > > passing only Value* has some cons and doesn't solve the general problem.
> > > > 1. it depends on SCEV
> > > > 2. currently we differentiate between strided and not strided access, but i think that there is more than those 2 cases so i thought creating some struct will be better approach
> > > I agree a Value* is not appropriate.
> > > I can live with either a struct or SCEV.
> > SCEV can't replace the struct, since you still need more info.
> > nullptr -> means default behavior
> > struct->isStrided = false -> means after check it is not strided at all
> > struct->isStrided = true -> you can look on the stride info or SCEV
> > but SCEV can be part of the struct if needed.
> > So in case there is no objections I prefer to go with struct which will contain bool and SCEV.
> But aren't you figuring isStrided out based on the SCEV?
> That is, if you pass a SCEV* S, it can be:
> nullptr -> default
> !isa<SCEVAddRecExpr>(S) -> not strided
> isa<SCEVAddRecExpr>(S) -> strided?
There are 4 cases that we want to handle
1) Default, when pointer to AddressInfo is null. This case is equal to the old variant when IsComplex was false;
2) The access is complex and we don't have info about the stride. In this case we put isStrided = false and do not check SCEV.
3) The access is strided (isStrided = true, SCEV is not null):
  (3.1) Stride is constant (SCEV is const int)
  (3.2) Stride is loop invariant unknown value (SCEV is not const int)

I agree that we can drop isStrided and say the following:
2) The access is complex and we don't have info about the stride. AddressInfo  is not null, but SCEV inside is null
3) The access is strided - SCEV is not null

Is that what you mean? I just think that the situation when "AddressInfo  is not null, but SCEV inside is null" looks strange from design point of view.


https://reviews.llvm.org/D27518