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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to