[PATCH] D67148: [LoopVectorize][PowerPC] Estimate int and float register pressure separately in loop-vectorize

[Zixuan Wu (Zeson) via Phabricator via llvm-commits]

wuzish marked an inline comment as done.
wuzish added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:801
+  /// return the target-provided register class for the provided type.
+  unsigned getRegisterClassForType(bool Vector, Type *Ty = nullptr) const;
+
----------------
hfinkel wrote:
> nhaehnle wrote:
> > hfinkel wrote:
> > > wuzish wrote:
> > > > arsenm wrote:
> > > > > I don't like spreading the concept of register classes corresponding to types.
> > > > > 
> > > > > I also don't think register classes as a concept should be leaking out to the IR
> > > > I think it's not the concept of register class in backend. It's an abstraction of register class in backend and just to classify and distinguish different kinds of data residing in different register position to help estimate register pressure.
> > > Yeah, I think that it's pretty important that these are *abstract* register classes - used to establish the mapping between the types of IR values and the number of simultaneous live ranges to which we'd like to limit for some set of those types - it's probably worth stating that explicitly in the description.
> > Can we explicitly call out the fact that these are not the CodeGen register classes?
> > 
> > Also, how about making the interface a `getRegisterClassForValue` as opposed to `getRegisterClassForType`? This would allow a context-sensitive determination of register classes. Specifically, in the AMDGPU backend, it would potentially allow us to distinguish between uniform and divergent values (in the overall program sense).
> > Can we explicitly call out the fact that these are not the CodeGen register classes?
> 
> I think that's a good idea. We can say that this is designed to provide a simple, high-level view of the register allocation later process later performed by the backend. These register classes don't necessarily map onto the register classes used by the backend.
> 
> > Also, how about making the interface a getRegisterClassForValue as opposed to getRegisterClassForType? This would allow a context-sensitive determination of register classes. Specifically, in the AMDGPU backend, it would potentially allow us to distinguish between uniform and divergent values (in the overall program sense).
> 
> I don't object, but we should make sure to think about the overall computational complexity of the analysis the backend would need to use, and whether this per-value interface will allow that analysis to be performed efficiently.
> 
if use `getRegisterClassForValue`, I find a scene to distinguish 2 different register class in PowerPC target. For the i1 type generated from icmp, there are 8 CRRC, but there are 32 CRBITRC for the i1 type generated from other operation. Is this required? @hfinkel 


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

https://reviews.llvm.org/D67148