On Wed, Aug 27, 2014 at 12:35 PM, Akira Hatanaka <[email protected]> wrote:
> The commit log in r166737 doesn't say much about why this is a warning > instead of an error, but I know there are cases where warnings are needed. > For example, clang has to issue warnings instead of errors for the > inline-asm statements in the test case committed in r216260. If it's not > desirable to change validateConstraintModifier, we can add a function which > checks the output size that is similar to validateInputSize in r167717 (see > attached patch), which was suggested in the post-commit review. > > > http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121112/067945.html > > I am not sure whether we can use fixit in this case. Fixit hints should be > used only if we know the user's intent and it's very clear that applying > the fixit hint is the right thing to do. Changing the type of variable "r" > to a 32-bit int will avoid crashing, but it doesn't look like that's what > the user wants. > In that case, the FixIt can be on a note attached to the diagnostic, rather than on the diagnostic itself. > On Mon, Aug 25, 2014 at 6:20 PM, Reid Kleckner <[email protected]> wrote: > >> 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
