[llvm] r258312 - Fixing bug in rL258132: [X86] Adding support for missing variations of X86 string related instructions

[Yatsina, Marina via llvm-commits]

I've mistakenly used assert("msg") ( --> the assert condition evaluates to "true" which causes NOT to assert) instead of assert (false && "msg") (--> which causes to always assert) or llvm_unreachable("msg").
Once that was fixed, 2 existent LIT tests have failed on the movsd overloaded meaning issue, so both flavors of the instruction are covered by existent tests.
Not sure how I can test the assert misuse, Diego uncovered it because he uses "-Wstring-conversion" flag.

Thanks,
Marina

-----Original Message-----
From: Quentin Colombet [mailto:qcolombet at apple.com] 
Sent: Wednesday, January 20, 2016 19:15
To: Yatsina, Marina
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r258312 - Fixing bug in rL258132: [X86] Adding support for missing variations of X86 string related instructions

Hi Marina,

Is there a chance you could produce a test case?

Thanks,
-Quentin
> On Jan 20, 2016, at 6:03 AM, Marina Yatsina via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: myatsina
> Date: Wed Jan 20 08:03:47 2016
> New Revision: 258312
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=258312&view=rev
> Log:
> Fixing bug in rL258132: [X86] Adding support for missing variations of 
> X86 string related instructions
> 
> There was a bug in my rL258132 because there's an overloading of the "movsd" and "cmpsd" instructions, e.g. movsd can be either "Move Data from String to String" (the case I wanted to handle) or "Move or Merge Scalar Double-Precision Floating-Point Value" (the case that causes the asserts).
> Added  code for escaping the unfamiliar scenarios and falling back to old behviour.
> Also changed the asserts to llvm_unreachable. 
> 
> 
> Modified:
>    llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
> 
> Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParse
> r/X86AsmParser.cpp?rev=258312&r1=258311&r2=258312&view=diff
> ======================================================================
> ========
> --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
> +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Wed Jan 20 
> +++ 08:03:47 2016
> @@ -1007,7 +1007,7 @@ std::unique_ptr<X86Operand> X86AsmParser bool 
> X86AsmParser::IsSIReg(unsigned Reg) {
>   switch (Reg) {
>   default:
> -    assert("Only (R|E)SI and (R|E)DI are expected!");
> +    llvm_unreachable("Only (R|E)SI and (R|E)DI are expected!");
>     return false;
>   case X86::RSI:
>   case X86::ESI:
> @@ -1024,7 +1024,7 @@ unsigned X86AsmParser::GetSIDIForRegClas
>                                           bool IsSIReg) {
>   switch (RegClassID) {
>   default:
> -    assert("Unexpected register class");
> +    llvm_unreachable("Unexpected register class");
>     return Reg;
>   case X86::GR64RegClassID:
>     return IsSIReg ? X86::RSI : X86::RDI; @@ -1090,6 +1090,10 @@ bool 
> X86AsmParser::VerifyAndAdjustOperan
>           RegClassID = X86::GR32RegClassID;
>         else if (X86MCRegisterClasses[X86::GR16RegClassID].contains(OrigReg))
>           RegClassID = X86::GR16RegClassID;
> +        else
> +          // Unexpexted register class type
> +          // Return false and let a normal complaint about bogus operands happen
> +          return false;
> 
>         bool IsSI = IsSIReg(FinalReg);
>         FinalReg = GetSIDIForRegClass(RegClassID, FinalReg, IsSI);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.