[llvm] r230585 - Remove a FIXME.

Eric Christopher echristo at gmail.com
Tue Mar 3 11:50:22 PST 2015


On Thu, Feb 26, 2015 at 11:49 PM David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Feb 26, 2015 at 11:45 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
>
>>
>> > On 2015 Feb 26, at 12:32, Eric Christopher <echristo at gmail.com> wrote:
>> >
>> >
>> >
>> > On Wed, Feb 25, 2015 at 5:38 PM Duncan P. N. Exon Smith <
>> dexonsmith at apple.com> wrote:
>> >
>> > > On 2015 Feb 25, at 16:00, Eric Christopher <echristo at gmail.com>
>> wrote:
>> > >
>> > > Author: echristo
>> > > Date: Wed Feb 25 18:00:35 2015
>> > > New Revision: 230585
>> > >
>> > > URL: http://llvm.org/viewvc/llvm-project?rev=230585&view=rev
>> > > Log:
>> > > Remove a FIXME.
>> > >
>> > > Explanation: This function is in TargetLowering because it uses
>> > > RegClassForVT which would need to be moved to TargetRegisterInfo
>> > > and would necessitate moving isTypeLegal over as well - a massive
>> > > change that would just require TargetLowering having a
>> TargetRegisterInfo
>> > > class member that it would use.
>> >
>> > Should this be in a doxygen comment in the header?
>> >
>> >
>> > Possibly not doxygen, but I can put it above the actual function?
>> >
>> > Thoughts?
>>
>> Either way seems fine to me -- maybe the comment isn't even useful.
>> It just occurred to me that someone might ask the same question in
>> the future, and `git blame` doesn't help much on missing lines ;).
>>
>
> *nod* I had the same thought
>
> (& I'd just go with a non-doxy comment wherever the FIXME was, since
> that's where it occurred to someone to wonder why it was the way it was)
>
>

I added it over the definition in TargetLoweringBase so that people on
other targets that have the same question can find the answer too without
looking at the X86 backend :)

r231111

-eric


>
>> >
>> > -eric
>> >
>> > >
>> > > Modified:
>> > >    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> > >
>> > > Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> > > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=230585&r1=230584&r2=230585&view=diff
>> > >
>> ==============================================================================
>> > > --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> > > +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Wed Feb 25 18:00:35
>> 2015
>> > > @@ -1930,7 +1930,6 @@ getPICJumpTableRelocBaseExpr(const Machi
>> > >   return MCSymbolRefExpr::Create(MF->getPICBaseSymbol(), Ctx);
>> > > }
>> > >
>> > > -// FIXME: Why this routine is here? Move to RegInfo!
>> > > std::pair<const TargetRegisterClass *, uint8_t>
>> > > X86TargetLowering::findRepresentativeClass(const TargetRegisterInfo
>> *TRI,
>> > >                                            MVT VT) const {
>> > >
>> > >
>> > > _______________________________________________
>> > > llvm-commits mailing list
>> > > llvm-commits at cs.uiuc.edu
>> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150303/fe51b199/attachment.html>


More information about the llvm-commits mailing list