[llvm-commits] Cost Table, take 2

[Renato Golin Linaro]

Nadav, Chandler,

Thanks for the reviews. I think we should start small, the new
Mega-Template-Class is too big and too specific for its own good. I'll just
move it inside lib/Target as suggested for now. We can make it better later
on.

cheers,
--renato


On 24 January 2013 17:27, Nadav Rotem <nrotem at apple.com> wrote:

>
> Renato spent way too much time on this. He is an ARM expert, but in the
> last week he spent his evenings writing and re-writing and re-writing
> different table implementations instead of focusing on what he initially
> planned to do. I am perfectly happy with any one of the solutions that
> Renato proposed. The LLVM-way is to get something in and refactor it later
> if needed. Chandler refactored the TargetTransformInfo layer and it is now
> awesome. If we need to refactor the cost-tables later then we can do
> it. Renato, please choose whatever implementation that you think is right.
> Chandler, please let us know if you see any major fouls or coding standard
> violations. Renato, I am grateful for the work that you did and I look
> forward to seeing the ARM cost model.
>
> Thanks,
> Nadav
>
> On Jan 24, 2013, at 1:32 AM, Chandler Carruth <chandlerc at google.com>
> wrote:
>
> On Thu, Jan 24, 2013 at 1:01 AM, Renato Golin Linaro <
> renato.golin at linaro.org> wrote:
>
>> On 24 January 2013 08:45, Chandler Carruth <chandlerc at google.com> wrote:
>>
>>> At this point, I'm not really seeing the benefit of this patch.
>>>
>>
>>  As usual, the patch is changing its original intent from reviews, so it
>> may well be a mess because it's trying to accommodate everyone's ideas.
>>
>> My original proposal was to *just* move the two cost structures out of
>> x86 code, so that the ARM code could use it, but many constraints appeared,
>> including making it a full ADT type. So, it's bigger than it should because
>> it may be expecting to work on areas it wasn't originally planned.
>>
>
> I'm sorry. I wasn't trying to push toward making it more generic in my
> comments. I wasn't aware of the previous iterations, thought you *wanted*
> it to be a generic ADT construct, and was trying to give ideas about how to
> go about that in the abstract... I didn't mean to seem like I was pushing
> for that particular solution.
>
>
>> 1) A TargetCostTable class template in include/llvm/Target which accepts
>>> a T type parameter that can be either MVT, std::pair<MVT, MVT>, or
>>> std::tuple<MVT, MVT, ...>. Then the ISD and the cost value can be concrete
>>> things.
>>>
>>
>> Yet another possible place, yet another slightly different implementation
>> detail... We're going in circles and I won't accommodate every one's
>> designs because they're all incompatible.
>>
>> The efforts in changing this have already outweighed its costs 10-fold.
>> I'm tempted to just leaving as it is and move the two tables and their two
>> functions with generic names to lib/Target and call it a day.
>>
>
> I'm sorry you're frustrated. I'm not trying to slow things down or make
> you accomodate everyone's wishes, I'm just trying to find a good
> abstraction and design for this code. Especially when it's going into a
> really low-level library like ADT or Support there is a really high bar in
> terms of design review...
>
> I wasn't really following the original review thread, so I can't comment
> about the other constraints or other directions. As an outsider to the
> discussion (and to the ARM backend entirely) it *seems* like your original
> proposal makes plenty of sense, but I just don't have the context there.
> Since Nadav is working on both of these cost models, maybe he can pick the
> design direction that makes the most sense?
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130124/2a3e605d/attachment.html>