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

["Cohen, Eugene" <eugene@hp.com>]

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