[llvm-commits] Trivial warning
Hal Finkel
hfinkel at anl.gov
Fri Nov 16 11:25:18 PST 2012
----- Original Message -----
> From: "David Blaikie" <dblaikie at gmail.com>
> To: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com>
> Cc: "Hal Finkel" <hfinkel at anl.gov>, llvm-commits at cs.uiuc.edu, "Joe Abbey" <jabbey at arxan.com>
> Sent: Friday, November 16, 2012 1:19:30 PM
> Subject: Re: [llvm-commits] Trivial warning
>
> On Fri, Nov 16, 2012 at 11:13 AM, Bill Schmidt
> <wschmidt at linux.vnet.ibm.com> wrote:
> > On Fri, 2012-11-16 at 12:32 -0600, Hal Finkel wrote:
> >> ----- Original Message -----
> >> > From: "Roman Divacky" <rdivacky at freebsd.org>
> >> > To: "Hal Finkel" <hfinkel at anl.gov>
> >> > Cc: llvm-commits at cs.uiuc.edu, "Joe Abbey" <jabbey at arxan.com>
> >> > Sent: Friday, November 16, 2012 12:13:53 PM
> >> > Subject: Re: [llvm-commits] Trivial warning
> >> >
> >> > On Fri, Nov 16, 2012 at 11:38:07AM -0600, Hal Finkel wrote:
> >> > > ----- Original Message -----
> >> > > > From: "Benjamin Kramer" <benny.kra at gmail.com>
> >> > > > To: "Joe Abbey" <jabbey at arxan.com>
> >> > > > Cc: "Hal Finkel" <hfinkel at anl.gov>, llvm-commits at cs.uiuc.edu
> >> > > > Sent: Friday, November 16, 2012 10:00:47 AM
> >> > > > Subject: Re: [llvm-commits] Trivial warning
> >> > > >
> >> > > >
> >> > > > On 16.11.2012, at 13:11, Joe Abbey <jabbey at arxan.com> wrote:
> >> > > >
> >> > > > > On Nov 16, 2012, at 12:38 AM, Joe Abbey <jabbey at arxan.com>
> >> > > > > wrote:
> >> > > > >
> >> > > > >> Hal (et al),
> >> > > > >>
> >> > > > >> One of my build bots reported a warning:
> >> > > > >> PPCRegisterInfo.cpp:501:51: warning: cast from type
> >> > > > >> ???const
> >> > > > >> llvm::MachineFunction*??? to type
> >> > > > >> ???llvm::MachineFunction*???
> >> > > > >> casts
> >> > > > >> away qualifiers [-Wcast-qual]
> >> > > > >> Index: lib/Target/PowerPC/PPCRegisterInfo.cpp
> >> > > > >> ===================================================================
> >> > > > >> --- lib/Target/PowerPC/PPCRegisterInfo.cpp (revision
> >> > > > >> 168110)
> >> > > > >> +++ lib/Target/PowerPC/PPCRegisterInfo.cpp (working
> >> > > > >> copy)
> >> > > > >> @@ -498,7 +498,8 @@
> >> > > > >> } else if (CRSpillFrameIdx) {
> >> > > > >> FrameIdx = CRSpillFrameIdx;
> >> > > > >> } else {
> >> > > > >> - MachineFrameInfo *MFI = ((MachineFunction
> >> > > > >> &)MF).getFrameInfo();
> >> > > > >> + MachineFrameInfo *MFI =
> >> > > > >> + (const_cast<MachineFunction
> >> > > > >> &>MF).getFrameInfo();
> >> > > > >> FrameIdx = MFI->CreateFixedObject((uint64_t)4,
> >> > > > >> (int64_t)-4,
> >> > > > >> true);
> >> > > > >> CRSpillFrameIdx = FrameIdx;
> >> > > > >> }
> >> > > > >>
> >> > > > >> Ok to commit?
> >> > > > >
> >> > > > > Past Joe,
> >> > > > >
> >> > > > > Make sure your current build is setup to do PowerPC
> >> > > > > builds.
> >> > > > >
> >> > > > > Also, swapping a warning for an error is a horrible
> >> > > > > trade???
> >> > > > > let's
> >> > > > > try that patch again:
> >> > > > >
> >> > > > > Index: lib/Target/PowerPC/PPCRegisterInfo.cpp
> >> > > > > ===================================================================
> >> > > > > --- lib/Target/PowerPC/PPCRegisterInfo.cpp (revision
> >> > > > > 168110)
> >> > > > > +++ lib/Target/PowerPC/PPCRegisterInfo.cpp (working
> >> > > > > copy)
> >> > > > > @@ -498,7 +498,8 @@
> >> > > > > } else if (CRSpillFrameIdx) {
> >> > > > > FrameIdx = CRSpillFrameIdx;
> >> > > > > } else {
> >> > > > > - MachineFrameInfo *MFI = ((MachineFunction
> >> > > > > &)MF).getFrameInfo();
> >> > > > > + MachineFrameInfo *MFI =
> >> > > > > + (const_cast<MachineFunction
> >> > > > > &>(MF)).getFrameInfo();
> >> > > > > FrameIdx = MFI->CreateFixedObject((uint64_t)4,
> >> > > > > (int64_t)-4,
> >> > > > > true);
> >> > > > > CRSpillFrameIdx = FrameIdx;
> >> > > > > }
> >> > > >
> >> > > > While this would be the right way to silence the warning,
> >> > > > the
> >> > > > whole
> >> > > > thing looks like a horrible hack to me. Shouldn't the
> >> > > > FrameIdx
> >> > > > generated in a place where MF isn't const?
> >> > >
> >> > > Roman, you had looked at these const issues, right? Do you
> >> > > have an
> >> > > opinion on this?
> >> >
> >> > I actively avoided any const_cast<>s in my const rage :)
> >> >
> >> > This code was contributed by wschmidt@ and iirc it needs to be
> >> > like this because PPC is special (addressing against fp).
> >>
> >> Bill, opinion?
> >>
> >
> > It looks to me like this has been changed a couple of times now:
> >
> > Original code:
> > MachineFrameInfo *MFI = ((MachineFunction
> > &)MF).getFrameInfo();
> >
> > Fixed in 165941 by Micah Villnow:
> >
> > 165941 mvillmow MachineFrameInfo *MFI =
> > (const_cast<MachineFunction &>(MF)).getFrameInfo();
> >
> > Reverted by Chandler in 167222:
> >
> > 167222 chandlerc MachineFrameInfo *MFI = ((MachineFunction
> > &)MF).getFrameInfo();
>
> It looks like this particular change was reverted without prejudice.
> The change was part of a much larger & more problematic change that
> was reverted on those merits - this was just collateral damage.
In that case, please (re-)apply the const_cast. Can someone please file a PR so that we don't forget to actually fix this problem.
Thanks again,
Hal
>
> >
> > And now it's under discussion again...
> >
> > I don't know of a better way to do this, so if anyone feels it's
> > too
> > much of a bloody hack, I welcome suggestions. Seems to me like the
> > const_cast is the best alternative, but it's probably worth asking
> > Chandler why he reverted Micah's change.
> >
> > My two cents,
> > Bill
> >
> >
> >
> >> >
> >> > I think Bill didnt see an easy way to fix this. I think
> >> > const_cast<>
> >> > in here is better than C style cast.
> >>
> >> Agreed.
> >>
> >> -Hal
> >>
> >> >
> >> > Roman
> >> >
> >>
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list