erichkeane marked 9 inline comments as done.
erichkeane 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);
----------------
rnk wrote:
> 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 }; };
Fair enough, thanks for the education guys!
https://reviews.llvm.org/D27529
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
