[PATCH] [AArch64] Refactor AArch64NamedImmMapper to become dependent on subtarget features.

[James Molloy]

Hi Eric,

Thanks for the elaboration. I now understand. Luckily these mapper classes
are always instantiated at the point of use anyway, but I agree that
changing their API to make it more clear their intended use would be a good
thing.

Vladimir, could you please remove the FeatureBits from the constructor of
these classes and have it passed in as a parameter to all the query
functions?

Cheers,

James

On Thu, 26 Mar 2015 at 23:51 Eric Christopher <echristo at gmail.com> wrote:

> On Thu, Mar 26, 2015 at 4:50 PM Eric Christopher <echristo at gmail.com>
> wrote:
>
>> On Thu, Mar 26, 2015 at 2:49 PM James Molloy <james at jamesmolloy.co.uk>
>> wrote:
>>
>>> Hi Eric,
>>>
>>> Are you saying the uint64_t feature sets shouldn't be copied into the
>>> helper class and perhaps should be passed to the query functions directly?
>>> Or that there's some API for querying them that I'm missing? Or something
>>> else?
>>>
>>>
>> I was thinking of instead just using the AsmParser's knowledge of such
>> things rather than copying them around and defining functions in each one.
>>
>> Ultimately no objection anymore, it'd be nice to not have state kept in
>> these helpers though.
>>
>>
> To elaborate:
>
> Because asm parsing and asm printing are both subtarget dependent and not
> target machine dependent. Some of this will end up changing and unifying
> uses of features will make it somewhat easier to clean up.
>
> (As an example take a look at the ARM inst printer and see where it checks
> on the subtarget and then realize that it's currently being set per module).
>
> -eric
>
>
>> -eric
>>
>>
>>> Sorry I'm bein
>>
>>
>>
>>
>>> g obtuse, but it seems like you have some context that I'm missing which
>>> is making it difficult for me to understand.
>>>
>>> Cheers,
>>>
>>> James
>>> On Thu, 26 Mar 2015 at 21:43, Eric Christopher <echristo at gmail.com>
>>> wrote:
>>>
>>>> A better question might be:
>>>>
>>>> Why are all of these classes copying the feature sets and using them
>>>> with their own API?
>>>>
>>>> -eric
>>>>
>>>> On Thu, Mar 26, 2015 at 2:39 PM James Molloy <james at jamesmolloy.co.uk>
>>>> wrote:
>>>>
>>>>> Hi Eric,
>>>>>
>>>>> Thanks for the clarification. I'm afraid I don't fully understand what
>>>>> you're saying though, so perhaps you could clarify further?
>>>>>
>>>>> The mapper classes map between a textual register/prefetch
>>>>> operation/barrier name and its binary equivalent. Whether this succeeds is
>>>>> sometimes dependent on a subtarget feature being available. My reading of
>>>>> this patch (and the code in situ) is that the mapper classes take a
>>>>> subtarget feature set and query it. The input comes transitively from
>>>>> ComputeAvailableFeatures(), so I'm afraid I don't understand what about
>>>>> this is "reimplementing".
>>>>>
>>>>> Sorry,
>>>>>
>>>>> James
>>>>>
>>>>> On Thu, 26 Mar 2015 at 19:53 Eric Christopher <echristo at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Why are you effectively reimplementing the feature sets for this
>>>>>> class?
>>>>>>
>>>>>> -eric
>>>>>>
>>>>>>
>>>>>> REPOSITORY
>>>>>>   rL LLVM
>>>>>>
>>>>>> http://reviews.llvm.org/D8496
>>>>>>
>>>>>> EMAIL PREFERENCES
>>>>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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/20150327/677de0c6/attachment.html>