[llvm-commits] [PATCH] Error cannot be reached

[Bob Wilson]

On May 12, 2011, at 10:10 PM, Cameron McInally wrote:

> Hey Chris,
> 
> The code in trunk looks like this:
> 
> if (R->getName() == "node" && !OpName.empty()) { //Let's call this (1)
>  if (OpName.empty()) //Let's call this (2)
>    error("'node' argument requires a name to match with operand list");
>  Args.push_back(OpName);
> }
> 
> Unless the expression OpName.empty() has side-effects, which I am fairly confident that it does not, the call cannot evaluate to false at (1) and true at (2). I'm assuming that '==' has a higher precedence than '&&' in C++, but maybe I'm mistaken. Is that not correct?

That logic is correct, except that the patch does not remove the unreachable code -- it changes the preceding conditional.  If OpName is empty, the previous code would skip over "Args.push_back(OpName)" without calling error().  With the patch, it will now produce an error().  It isn't at all obvious that is the right thing to do.

If you just want to make an obvious change to remove the dead code, then remove the second conditional (2)  and the call to error().  Otherwise, you need to do some more investigation and/or explaining.

> 
> Essentially, this code sequence is:
> 
> bool b = true;
> if(b) {
>  if(!b) {
>    printf("unreachable\n");
>  }

> }
> 
> Cameron
> 
> ________________________________________
> From: Chris Lattner [clattner at apple.com]
> Sent: Thursday, May 12, 2011 11:18 PM
> To: Cameron McInally
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Error cannot be reached
> 
> On May 12, 2011, at 2:47 PM, Cameron McInally wrote:
> 
>> A silly little patch for unreachable code. It's self-explanatory:
> 
> How is that unreachable?  OpName != R->getName()
> 
> -Chris
> 
>> 
>> mcinally/llvm> svn diff
>> Index: utils/TableGen/CodeGenDAGPatterns.cpp
>> ===================================================================
>> --- utils/TableGen/CodeGenDAGPatterns.cpp    (revision 131245)
>> +++ utils/TableGen/CodeGenDAGPatterns.cpp    (working copy)
>> @@ -1732,7 +1732,7 @@
>> 
>>    // Input argument?
>>    TreePatternNode *Res = new TreePatternNode(DI, 1);
>> -    if (R->getName() == "node" && !OpName.empty()) {
>> +    if (R->getName() == "node") {
>>      if (OpName.empty())
>>        error("'node' argument requires a name to match with operand
>> list");
>>      Args.push_back(OpName);
>> 
>> mcinally/llvm> svn info
>> Path: .
>> URL: http://llvm.org/svn/llvm-project/llvm/trunk
>> Repository Root: http://llvm.org/svn/llvm-project
>> Repository UUID: 91177308-0d34-0410-b5e6-96231b3b80d8
>> Revision: 131241
>> Node Kind: directory
>> Schedule: normal
>> Last Changed Author: evancheng
>> Last Changed Rev: 131241
>> Last Changed Date: 2011-05-12 15:30:01 -0500 (Thu, 12 May 2011)
>> 
>> Thanks,
>> Cameron
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits