Re: I've submitted 207175 for a clang 3.8.0 va_list handling problem for powerpc [fpr use also tested]

[Mark Millard]

On 2016-Feb-21, at 1:02 AM, Roman Divacky  wrote:
> 
> On Sat, Feb 20, 2016 at 03:26:58PM +0100, Dimitry Andric wrote:
>> On 20 Feb 2016, at 09:34, Roman Divacky  wrote:
>>> Fwiw, I've just committed the patch to clang in r261422. You might want
>>> to keep using a local modification or ask dim@ to import that patch
>>> to our copy of 3.8.
>> 
>> I've asked the LLVM release manager to consider merging this into the
>> 3.8 branch.  The fix looks trivial enough. :)
>> 
>> -Dimitry
>> 
> 
> Cool :)
> 
> Mark, so what remains broken now beside the C++ exceptions? Also, can you try
> to take a look at kernel?
> 
> Thanks! Roman


Implication of the below detailed answer: I've not thought about the kernel 
much so far and it may well be some time before I do.


Getting each thing fixed/operable/improved/worked-around for world/userland has 
helped me explore finding the next thing. The C++ exception problems currently 
block using "kyua -k /usr/tests/Kyuafile", something I've been hoping to be 
able to do as evidence for how much is (then) working based on a clang 3.8.0 
buildworld. I've been sticking to providing evidence for details world/userland 
issues before tackling anything else. (So far I'm not generally fixing things, 
mostly just finding evidence of problem details.)

You may not know that I run PowerPC (32-bit) with modified signal delivery: my 
adjustment uses a so-called "red-zone" to avoid the incorrect timing of the 
stack pointer adjustments that clang 3.8.0's code generation produces. (An ABI 
violation that I've worked around for world/userland.) The work around was 
required to be able to have a self-hosted buildworld's complete without make 
getting SEGV's/BusError's that stop the build.

No one is working on clang 3.8.0's 32-bit PowerPC stack-pointer-handling ABI 
violations so far as I know.

I doubt anyone will tackle the kernel overall for 32-bit PowerPC as long as the 
stack pointer is decremented late and incremented early in clang's generated 
code. I expect that handling such is comparatively simple in world/userland 
(see above) compared to in the kernel and the kernel's handling of is own stack.

I doubt that FreeBSD would make even the red-zone code change for 
world/userland official. It is operationally compatible with the official ABI 
in world/userland but is temporarily somewhat wasteful of some stack bytes. But 
mostly it is just something not required by the official ABI and the signal 
delivery change does not help the bigger/messier kernel-stack handling issue.

I doubt FreeBSD would ever officially split buildworld and buildkernel by using 
different compilers as I have, even temporarily. So no official PowerPC clang 
context until everything works for both parts would be my guess. I just view 
the split as a development/testing technique for helping find details of 
problems in the simpler world/userland context first.



I did once take a quick look at clang 3.8.0 use in buildkernel instead of using 
gcc 4.2.1. I do not remember all the details but one thing that I remember is 
that clang's integrated-assembler notation and the kernel's source files did 
not agree in various places.

I have no formal descriptions of the two assembler notations or description of 
how they correspond where different. This tends to prevent any systematic 
process for such issues. (I'm no PowerPC assembler expert: I look things up as 
I go.)

If I remember the count right I also identified two other kernel 
points-for-investigation very quickly and stopped there in order to continue 
with world/userland investigations. I was guessing there was a lot more to find 
relative to the kernel based on how quickly those 3 subject areas were found.


Since I started exploring FreeBSD I've observed that things happen without 
prior notice that suspend my FreeBSD activity for weeks or months at a time. 
That has happened several times. So I'm not sure how far I'll get before such 
happens again.

And PowerPC access has that issue even more: I can end up without access to the 
old PowerMacs for long periods even if I can spend some time exploring some 
(other) aspects of FreeBSD at the time.

At this point I've no clue how I'll find what specific details are involved in 
the "clang 3.8.0 compiled C++ exception infrastructure vs. 
clang++380/g++5/g++49/g++421 compiled code" mismatch. I've no clue how long 
that will take.



So, overall, I'm not ready to think much about the kernel yet.


===
Mark Millard
markmi at dsl-only.net

___
freebsd-toolchain@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-toolchain
To unsubscribe, send any mail to "freebsd-toolchain-unsubscr...@freebsd.org"

Re: I've submitted 207175 for a clang 3.8.0 va_list handling problem for powerpc [fpr use also tested]

[Dimitry Andric]

On 20 Feb 2016, at 09:34, Roman Divacky  wrote:
> Fwiw, I've just committed the patch to clang in r261422. You might want
> to keep using a local modification or ask dim@ to import that patch
> to our copy of 3.8.

I've asked the LLVM release manager to consider merging this into the
3.8 branch.  The fix looks trivial enough. :)

-Dimitry



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: I've submitted 207175 for a clang 3.8.0 va_list handling problem for powerpc [fpr use also tested]

[Mark Millard]

Thanks!

llvm bugzilla's 26605 did not having anything yet for this so I've copied over 
your note. But I've left the status alone.


The next thing that I ran into looks nastier: c++'s exception handling is 
broken.

#include 

int main(void)
{
try { throw std::exception(); }
catch (std::exception& e) {} // same result without &
return 0;
}


does not work on powerpc (SEGV) or powerpc64 (unbounded loop, never returning 
from _Unwind_RaiseException). (The powerpc64 context is using 
devel/powerpc64-gcc or g++49 as the compiler with the system's headers and 
libraries. powerpc64-gcc was used for buildworld/buildkernel as well for this 
context.)

[g++49 using its own headers and libraries works fine for the above program.]


===
Mark Millard
markmi at dsl-only.net

On 2016-Feb-20, at 12:34 AM, Roman Divacky  wrote:

Fwiw, I've just committed the patch to clang in r261422. You might want
to keep using a local modification or ask dim@ to import that patch
to our copy of 3.8.

Thanks for your diagnosis and testing!

Roman

On Thu, Feb 18, 2016 at 02:29:29PM -0800, Mark Millard wrote:
> On 2016-Feb-17, at 9:23 PM, Mark Millard  wrote:
>> 
>> My fpr related notes/worries about the fix were wrong.
>> 
>> I finally got some time to look at this again and I see that I somehow 
>> missed the following code when I looked before:
>> 
>> // The calling convention either uses 1-2 GPRs or 1 FPR.
>> Address NumRegsAddr = Address::invalid();
>> if (isInt || IsSoftFloatABI) {
>>   NumRegsAddr = Builder.CreateStructGEP(VAList, 0, CharUnits::Zero(), "gpr");
>> } else {
>>   NumRegsAddr = Builder.CreateStructGEP(VAList, 1, CharUnits::One(), "fpr");
>> }
>> 
>> So the
>> 
>>   Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
>> 
>> in Case 2 is tracking gpr vs. fpr usage contexts as it should. Also:
>> 
>> llvm::Value *NumRegs = Builder.CreateLoad(NumRegsAddr, "numUsedRegs");
>> 
>> // "Align" the register count when TY is i64.
>> if (isI64 || (isF64 && IsSoftFloatABI)) {
>>   NumRegs = Builder.CreateAdd(NumRegs, Builder.getInt8(1));
>>   NumRegs = Builder.CreateAnd(NumRegs, Builder.getInt8((uint8_t) ~1U));
>> }
>> 
>> llvm::Value *CC =
>> Builder.CreateICmpULT(NumRegs, Builder.getInt8(8), "cond");
>> 
>> is using the same bounds check figure (8) for gpr and fpr.
>> 
>> Apparently that common bound is one reason that the clang numbering is not 
>> the same as the ABI document's numbering: clang's numbering allows using the 
>> same figure for both contexts. (Given the prior alignment for isI64 (or 
>> isF64 with IsSoftFloatABI).)
>> 
>> Sorry for the prior noise about fpr.
>> 
>> It is still true that DOUBLE_OR_FLOAT is untested so far.
>> 
>> ===
>> Mark Millard
>> markmi at dsl-only.net
> 
> I finally got some time to apply to some basic testing involving double as 
> well (for involving fpr use). . .
> 
> No problems with exceptions. Looking at the memory contents at various stages 
> in gdb looks good. va_list's gpr, fpr, overflow_arg_area changes as its 
> va_args use progresses look good. Values extracted by va_args use looks good. 
> Both default and -O2.
> 
> The added
> 
>> Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
> 
> 
> passes my checks. I've not observed any problems from buildworld materials, 
> unlike when that line is missing.
> 
> [Note: I run with the signal delivery modified to have a "red zone" to deal 
> with other aspects of clang 3.8.0 code generation that are not ABI compliant 
> for when the stack pointer is moved. Having a "red zone" is still 
> operationally correct for an ABI compliant code generation, it just 
> temporarily wastes more bytes. Also: the kernel was built with gcc 4.2.1 but 
> world was built with clang 3.8.0.]
> 
> 
> ===
> Mark Millard
> markmi at dsl-only.net
> 
> . . . [bad fpr related material omitted] . . .
> 
> On 2016-Feb-16, at 2:45 AM, Mark Millard  wrote:
> 
> I used:
> 
>> Index: /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp
>> ===
>> --- /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp 
>> (revision 295601)
>> +++ /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp 
>> (working copy)
>> @@ -3569,6 +3569,8 @@
>> {
>>  CGF.EmitBlock(UsingOverflow);
>> 
>> +Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
>> +
>>  // Everything in the overflow area is rounded up to a size of at least 4.
>>  CharUnits OverflowAreaAlign = CharUnits::fromQuantity(4);
> 
> as my test change. (Get evidence of operation before potential cleanup of the 
> duplicated 8's.)
> 
> After a full buildworld/installworld based on the updated compiler. . .
> 
> My simple example of the problem no longer fails.
> 
> "ls -l -n /" now works.
> 
> "svnlite update -r295601 /usr/src" now works.
> 
> So whatever you want to do for the details of any submitted code, the basics 
> of the change do avoid the 

Re: I've submitted 207175 for a clang 3.8.0 va_list handling problem for powerpc [fpr use also tested]

[Mark Millard]

On 2016-Feb-17, at 9:23 PM, Mark Millard  wrote:
> 
> My fpr related notes/worries about the fix were wrong.
> 
> I finally got some time to look at this again and I see that I somehow missed 
> the following code when I looked before:
> 
>  // The calling convention either uses 1-2 GPRs or 1 FPR.
>  Address NumRegsAddr = Address::invalid();
>  if (isInt || IsSoftFloatABI) {
>NumRegsAddr = Builder.CreateStructGEP(VAList, 0, CharUnits::Zero(), "gpr");
>  } else {
>NumRegsAddr = Builder.CreateStructGEP(VAList, 1, CharUnits::One(), "fpr");
>  }
> 
> So the
> 
>Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
> 
> in Case 2 is tracking gpr vs. fpr usage contexts as it should. Also:
> 
>  llvm::Value *NumRegs = Builder.CreateLoad(NumRegsAddr, "numUsedRegs");
> 
>  // "Align" the register count when TY is i64.
>  if (isI64 || (isF64 && IsSoftFloatABI)) {
>NumRegs = Builder.CreateAdd(NumRegs, Builder.getInt8(1));
>NumRegs = Builder.CreateAnd(NumRegs, Builder.getInt8((uint8_t) ~1U));
>  }
> 
>  llvm::Value *CC =
>  Builder.CreateICmpULT(NumRegs, Builder.getInt8(8), "cond");
> 
> is using the same bounds check figure (8) for gpr and fpr.
> 
> Apparently that common bound is one reason that the clang numbering is not 
> the same as the ABI document's numbering: clang's numbering allows using the 
> same figure for both contexts. (Given the prior alignment for isI64 (or isF64 
> with IsSoftFloatABI).)
> 
> Sorry for the prior noise about fpr.
> 
> It is still true that DOUBLE_OR_FLOAT is untested so far.
> 
> ===
> Mark Millard
> markmi at dsl-only.net

I finally got some time to apply to some basic testing involving double as well 
(for involving fpr use). . .

No problems with exceptions. Looking at the memory contents at various stages 
in gdb looks good. va_list's gpr, fpr, overflow_arg_area changes as its va_args 
use progresses look good. Values extracted by va_args use looks good. Both 
default and -O2.

The added

> Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);


passes my checks. I've not observed any problems from buildworld materials, 
unlike when that line is missing.

[Note: I run with the signal delivery modified to have a "red zone" to deal 
with other aspects of clang 3.8.0 code generation that are not ABI compliant 
for when the stack pointer is moved. Having a "red zone" is still operationally 
correct for an ABI compliant code generation, it just temporarily wastes more 
bytes. Also: the kernel was built with gcc 4.2.1 but world was built with clang 
3.8.0.]


===
Mark Millard
markmi at dsl-only.net

. . . [bad fpr related material omitted] . . .

On 2016-Feb-16, at 2:45 AM, Mark Millard  wrote:

I used:

> Index: /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp
> ===
> --- /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp  
> (revision 295601)
> +++ /usr/src/contrib/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp  
> (working copy)
> @@ -3569,6 +3569,8 @@
> {
>   CGF.EmitBlock(UsingOverflow);
> 
> +Builder.CreateStore(Builder.getInt8(8), NumRegsAddr);
> +
>   // Everything in the overflow area is rounded up to a size of at least 4.
>   CharUnits OverflowAreaAlign = CharUnits::fromQuantity(4);

as my test change. (Get evidence of operation before potential cleanup of the 
duplicated 8's.)

After a full buildworld/installworld based on the updated compiler. . .

My simple example of the problem no longer fails.

"ls -l -n /" now works.

"svnlite update -r295601 /usr/src" now works.

So whatever you want to do for the details of any submitted code, the basics of 
the change do avoid the SEGVs and allow these programs to work.


===
Mark Millard
markmi at dsl-only.net

On 2016-Feb-15, at 4:27 PM, Mark Millard  wrote:

On 2016-Feb-15, at 1:20 PM, Mark Millard  wrote:
> 
> On 2016-Feb-15, at 12:18 PM, Roman Divacky  wrote:
>> 
>> On Mon, Feb 15, 2016 at 12:17:50PM -0800, Mark Millard wrote:
>>> On 2016-Feb-15, at 11:11 AM, Roman Divacky  wrote:
 
 Mark, I believe you're right. What do you think about this patch?
 
 Index: tools/clang/lib/CodeGen/TargetInfo.cpp
 ===
 --- tools/clang/lib/CodeGen/TargetInfo.cpp (revision 260852)
 +++ tools/clang/lib/CodeGen/TargetInfo.cpp (working copy)
 @@ -3599,6 +3599,8 @@
 {
 CGF.EmitBlock(UsingOverflow);
 
 +Builder.CreateStore(Builder.getInt8(11), NumRegsAddr);
 +
 // Everything in the overflow area is rounded up to a size of at least 4.
 CharUnits OverflowAreaAlign = CharUnits::fromQuantity(4);
 
 
 Can you test it?
>>> 
>>> It may be later today before I can start the the test process.
>>> 
>>> While your change is not wrong as presented, it does seem to be based on 
>>> the ABI document's numbering with the range 3 <= gr <12, where 3