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

[Pete Batard <pete@akeo.ie>]

Hi Ard. Thanks for reviewing this.

On 2017.12.12 19:59, Ard Biesheuvel wrote:
>> +  EXPORT _fltused
>> +  EXPORT __brkdiv0
>> +
> 
> Why are these being exported? Are these part of the CRT ABI?

Good point. I exported them because I was encountering undefined symbols 
for those when I started working on adding ARM support. But I now 
suspect they may have come from an export statement from VS headers 
(which we no longer include), as re-test shows that the exports seem to 
be superfluous.

I will test further to confirm that this is the case, and remove the 
unneeded exports.

>> +__brkdiv0
>> +    udf #249
>> +
> 
> This will cause a crash when invoked, in a way that gives very little
> to go on to figure out what happens.

Yes. I mostly copied what ReactOS was doing here, since it should be 
similar to what Microsoft does internally.

> Perhaps branch to a C function
> here that calls ASSERT() and CpuDeadLoop ?

I don't mind giving it a try, if we keep the separate .asm files for MSVC.

On the other hand, if I am to try to reuse existing assembly, as you 
suggest below, then I don't think I want to introduce anything that will 
differ from how divisions by zero are currently handled there. As far as 
I can tell (for RVCT, which is the only assembly that might be suitable 
for reuse with MSFT) the current way of handling a zero divisor is to 
set result and remainder to 0 and return (as per uldiv.asm). For gcc, we 
also have the following comment in div.S:

   "@ What to do about division by zero?  For now, just return."

So, while I agree that calling ASSERT and CpuDeadLoop is probably the 
better approach, I'd rather have the introduction of this behaviour be 
the subject of a separate ARM harmonization patch, that applies to all 
the toolchains, rather than something that's specific to Visual Studio 
usage.

> There are many different division implementations already in the same
> directory. Do we really need a new one?

I'll look into that. My goal with this series was to make sure that the 
changes being introduced would not break existing toolchains, which is 
why I saw it preferable to keep the MSVC intrinsics separate.

Now, gcc assembly is not something that can be reused easily with the MS 
toolchain, so the only possibility are the RVCT .asm files.

There's a fair bit of work involved into trying to figure out if and how 
we can reuse that code, but I'll see what I can do. It'll probably be a 
few weeks before I get back to this list on that however.

Regards,

/Pete