Can you investigate why we are warning in the first place? I think we
should either only warn or only error. Currently we have a warning with a
fixit but we don't recover as though we had applied the fixit. If we did
that, we would not crash.

In addition to the Clang-side changes, LLVM should probably be returning an
error or reporting a fatal error instead of hitting unreachable.


On Mon, Aug 25, 2014 at 2:10 PM, Akira Hatanaka <[email protected]> wrote:

> Rebased patches attached.
>
> I also made changes to the clang patch so that clang can error-out after a
> size mismatch is found as soon as
> possible.TargetInfo::validateConstraintModifier has an extra parameter
> IsError, which is set when it decides there is no point in continuing
> compilation and it should stop compilation immediately. The error message
> clang prints looks better than lllvm's message, but if it isn't right to
> change the warning to an error, then I guess we have to detect the error
> later just before isel, as is done in the llvm patch.
>
> On Wed, Jul 23, 2014 at 3:56 PM, Akira Hatanaka <[email protected]>
> wrote:
>
>> llvm should error-out when a 64-bit variable is bound to a single
>> register in x86 32-bit mode, but ToT clang/llvm fails to detect this error
>> and continues compilation until it crashes in type-legalization:
>>
>> $ llc test/CodeGen/X86/inline-asm-regsize.ll  -O3
>> -mtriple=i386-apple-darwin -o -
>>
>> inline-asm-regsize.ll  -O3 -mtriple=i386-apple-darwin -o -
>>
>> .section __TEXT,__text,regular,pure_instructions
>>
>> ExpandIntegerResult #0: 0x7fa2d1041728: i64 = Register %RCX [ID=0]
>>
>>
>> Do not know how to expand the result of this operator!
>>
>> UNREACHABLE executed at
>> /Users/ahatanaka/projects/llvm/git/llvm3/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:1116!
>>
>> The attached patch fixes llvm to error-out and print this error message:
>>
>> error: Cannot bind a variable larger than 32-bit to a single register in
>> 32-bit mode
>>
>>
>> My initial solution was to have clang detect this error in
>> TargetInfo::validateConstraintModifier. However, the code in
>> SemaStmtAsm.cpp has to be changed to error-out instead of issuing a
>> warning, which I wasn't sure was the right thing to do. I am attaching this
>> patch too in case someone has a suggestion or an opinion on it.
>>
>> <rdar://problem/17476970>
>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to