[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: lib/CodeGen/TargetInfo.cpp:3871
+    for (auto &I : FI.arguments()) {
+      if (Count < 6)
+        I.info = classify(I.type, FreeSSERegs, false, IsVectorCall, IsRegCall);
----------------
majnemer wrote:
> erichkeane wrote:
> > majnemer wrote:
> > > majnemer wrote:
> > > > And here,
> > > I was OK with `6` showing up in prior code because there was a comment but there isn't such a comment down here.  Maybe we should have this `6` live in an enum?
> > Thats a good point.  Any reason it couldn't just be a static constant in each of the ABIInfos?  Or am I missing your point?
> It could be but LLVM usually prefers enums over class-scope static variables.  They require an awkward definition somewhere should they be ODR used.
I think some of us who have spent time implementing the C++ ABI rules around static const int data members feel that you should use an enumerator in an unnamed enum instead because the ABI rules are easier to follow. With static data members, you sometimes have to worry about providing a definition like so:
  struct A { static const int kThing = 42; }; // in a .h
  const int A::kThing; // in some .cc file, needed "sometimes"
Simpler:
  struct A { enum : int { kThing = 42 }; };


https://reviews.llvm.org/D27529