Re: [PATCH v4 4/6] ArmPkg/Library/CompilerIntrinsicsLib: Enable VS2017/ARM builds

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 11 January 2018 at 16:56, Cohen, Eugene <eugene@hp.com> wrote:
> Pete,
>
>> How about I modify the patch to use "AREA s_<original_func_name>"
>> instead of "AREA Math" as per the current proposal?
>
> That's how it used to work before the macro was introduced, per commit dcb2e4bb61931e2dee1739bb76aba315002f0a82 two years ago.
>
> I personally have no problem going back to the individual AREA s_<name>  approach now that we have a good reason to do so.
>
>> Also, you'll notice that the current div.asm [1], which is used by RVCT does
>> *not* rely on the macro, so, unless this is intentional, there already seems
>> to exist inconsistencies with regards to using the RVCT_ASM_EXPORT
>> macro to ensure the removal of dead code...
>
> I think this was likely an oversight.
>
>> So, to summarise, I would much prefer if we could keep most of the
>> current patch, and simply use the following where needed:
>>
>> AREA  s___aeabi_ldivmod, CODE, READONLY, ARM AREA  s___aeabi_llsr,
>> CODE, READONLY, ARM AREA  s___aeabi_uldivmod, CODE, READONLY,
>> ARM
>>
>
> Agreed, this is fine so long as we agree on the definition of "where needed".  In general I would expect each independent assembly function to have its own AREA directive (e.g. math functions).  In some cases there will clearly be a collection of dependent functions that would be better served by a single area directive (e.g. MMU initialization functions).
>
> Much Thanks!
>
> Eugene
>

Agreed.