<div class="gmail_quote">On Fri, Nov 16, 2012 at 11:13 AM, Bill Schmidt <span dir="ltr"><<a href="mailto:wschmidt@linux.vnet.ibm.com" target="_blank">wschmidt@linux.vnet.ibm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Fri, 2012-11-16 at 12:32 -0600, Hal Finkel wrote:<br>
> ----- Original Message -----<br>
> > From: "Roman Divacky" <<a href="mailto:rdivacky@freebsd.org">rdivacky@freebsd.org</a>><br>
> > To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> > Cc: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>, "Joe Abbey" <<a href="mailto:jabbey@arxan.com">jabbey@arxan.com</a>><br>
> > Sent: Friday, November 16, 2012 12:13:53 PM<br>
> > Subject: Re: [llvm-commits] Trivial warning<br>
> ><br>
> > On Fri, Nov 16, 2012 at 11:38:07AM -0600, Hal Finkel wrote:<br>
> > > ----- Original Message -----<br>
> > > > From: "Benjamin Kramer" <<a href="mailto:benny.kra@gmail.com">benny.kra@gmail.com</a>><br>
> > > > To: "Joe Abbey" <<a href="mailto:jabbey@arxan.com">jabbey@arxan.com</a>><br>
> > > > Cc: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>, <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > > > Sent: Friday, November 16, 2012 10:00:47 AM<br>
> > > > Subject: Re: [llvm-commits] Trivial warning<br>
> > > ><br>
> > > ><br>
> > > > On 16.11.2012, at 13:11, Joe Abbey <<a href="mailto:jabbey@arxan.com">jabbey@arxan.com</a>> wrote:<br>
> > > ><br>
> > > > > On Nov 16, 2012, at 12:38 AM, Joe Abbey <<a href="mailto:jabbey@arxan.com">jabbey@arxan.com</a>><br>
> > > > > wrote:<br>
> > > > ><br>
> > > > >> Hal (et al),<br>
> > > > >><br>
> > > > >> One of my build bots reported a warning:<br>
> > > > >> PPCRegisterInfo.cpp:501:51: warning: cast from type ???const<br>
> > > > >> llvm::MachineFunction*??? to type ???llvm::MachineFunction*???<br>
> > > > >> casts<br>
> > > > >> away qualifiers [-Wcast-qual]<br>
> > > > >> Index: lib/Target/PowerPC/PPCRegisterInfo.cpp<br>
> > > > >> ===================================================================<br>
> > > > >> --- lib/Target/PowerPC/PPCRegisterInfo.cpp   (revision 168110)<br>
> > > > >> +++ lib/Target/PowerPC/PPCRegisterInfo.cpp   (working copy)<br>
> > > > >> @@ -498,7 +498,8 @@<br>
> > > > >>      } else if (CRSpillFrameIdx) {<br>
> > > > >>        FrameIdx = CRSpillFrameIdx;<br>
> > > > >>      } else {<br>
> > > > >> -      MachineFrameInfo *MFI = ((MachineFunction<br>
> > > > >> &)MF).getFrameInfo();<br>
> > > > >> +      MachineFrameInfo *MFI =<br>
> > > > >> +        (const_cast<MachineFunction &>MF).getFrameInfo();<br>
> > > > >>        FrameIdx = MFI->CreateFixedObject((uint64_t)4,<br>
> > > > >>        (int64_t)-4,<br>
> > > > >>        true);<br>
> > > > >>        CRSpillFrameIdx = FrameIdx;<br>
> > > > >>      }<br>
> > > > >><br>
> > > > >> Ok to commit?<br>
> > > > ><br>
> > > > > Past Joe,<br>
> > > > ><br>
> > > > > Make sure your current build is setup to do PowerPC builds.<br>
> > > > ><br>
> > > > > Also, swapping a warning for an error is a horrible trade???<br>
> > > > > let's<br>
> > > > > try that patch again:<br>
> > > > ><br>
> > > > > Index: lib/Target/PowerPC/PPCRegisterInfo.cpp<br>
> > > > > ===================================================================<br>
> > > > > --- lib/Target/PowerPC/PPCRegisterInfo.cpp    (revision 168110)<br>
> > > > > +++ lib/Target/PowerPC/PPCRegisterInfo.cpp    (working copy)<br>
> > > > > @@ -498,7 +498,8 @@<br>
> > > > >      } else if (CRSpillFrameIdx) {<br>
> > > > >        FrameIdx = CRSpillFrameIdx;<br>
> > > > >      } else {<br>
> > > > > -      MachineFrameInfo *MFI = ((MachineFunction<br>
> > > > > &)MF).getFrameInfo();<br>
> > > > > +      MachineFrameInfo *MFI =<br>
> > > > > +        (const_cast<MachineFunction &>(MF)).getFrameInfo();<br>
> > > > >        FrameIdx = MFI->CreateFixedObject((uint64_t)4,<br>
> > > > >        (int64_t)-4,<br>
> > > > >        true);<br>
> > > > >        CRSpillFrameIdx = FrameIdx;<br>
> > > > >      }<br>
> > > ><br>
> > > > While this would be the right way to silence the warning, the<br>
> > > > whole<br>
> > > > thing looks like a horrible hack to me. Shouldn't the FrameIdx<br>
> > > > generated in a place where MF isn't const?<br>
> > ><br>
> > > Roman, you had looked at these const issues, right? Do you have an<br>
> > > opinion on this?<br>
> ><br>
> > I actively avoided any const_cast<>s in my const rage :)<br>
> ><br>
> > This code was contributed by wschmidt@ and iirc it needs to be<br>
> > like this because PPC is special (addressing against fp).<br>
><br>
> Bill, opinion?<br>
><br>
<br>
</div></div>It looks to me like this has been changed a couple of times now:<br>
<br>
Original code:<br>
<div class="im">      MachineFrameInfo *MFI = ((MachineFunction &)MF).getFrameInfo();<br>
<br>
</div>Fixed in 165941 by Micah Villnow:<br>
<br>
165941   mvillmow       MachineFrameInfo *MFI =<br>
(const_cast<MachineFunction &>(MF)).getFrameInfo();<br>
<br>
Reverted by Chandler in 167222:<br>
<br>
167222  chandlerc       MachineFrameInfo *MFI = ((MachineFunction<br>
&)MF).getFrameInfo();<br>
<br>
And now it's under discussion again...<br>
<br>
I don't know of a better way to do this, so if anyone feels it's too<br>
much of a bloody hack, I welcome suggestions.  Seems to me like the<br>
const_cast is the best alternative, but it's probably worth asking<br>
Chandler why he reverted Micah's change.<br>
<br>
My two cents,<br>
Bill<br>
<div class="im HOEnZb"><br></div></blockquote><div><br>Looks like Chandler's revert was part of a revert of a large patch from Micah. So I don't think he specifically targeted the const_cast with the revert.<br> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im HOEnZb">
<br>
<br>
> ><br>
> > I think Bill didnt see an easy way to fix this. I think const_cast<><br>
> > in here is better than C style cast.<br>
><br>
> Agreed.<br>
><br>
>  -Hal<br>
><br>
> ><br>
> > Roman<br>
> ><br>
><br>
<br>
</div><div class="HOEnZb"><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>~Craig<br>