[llvm-commits] [llvm] r166157 - /llvm/trunk/lib/VMCore/Verifier.cpp

[Bill Wendling]

n Oct 17, 2012, at 5:26 PM, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Wed, Oct 17, 2012 at 5:03 PM, Bill Wendling <isanbard at gmail.com> wrote:
>> On Oct 17, 2012, at 5:01 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> 
>>> On Wed, Oct 17, 2012 at 4:54 PM, Bill Wendling <isanbard at gmail.com> wrote:
>>>> Author: void
>>>> Date: Wed Oct 17 18:54:19 2012
>>>> New Revision: 166157
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=166157&view=rev
>>>> Log:
>>>> Check that the operand of the GEP is not the GEP itself. This occurred during an LTO build of LLVM.
>>>> 
>>>> Modified:
>>>>   llvm/trunk/lib/VMCore/Verifier.cpp
>>>> 
>>>> Modified: llvm/trunk/lib/VMCore/Verifier.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Verifier.cpp?rev=166157&r1=166156&r2=166157&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/VMCore/Verifier.cpp (original)
>>>> +++ llvm/trunk/lib/VMCore/Verifier.cpp Wed Oct 17 18:54:19 2012
>>>> @@ -1371,9 +1371,11 @@
>>>>  Type *TargetTy = GEP.getPointerOperandType()->getScalarType();
>>>> 
>>>>  Assert1(isa<PointerType>(TargetTy),
>>>> -    "GEP base pointer is not a vector or a vector of pointers", &GEP);
>>>> +          "GEP base pointer is not a vector or a vector of pointers", &GEP);
>>>>  Assert1(cast<PointerType>(TargetTy)->getElementType()->isSized(),
>>>>          "GEP into unsized type!", &GEP);
>>>> +  Assert1(GEP.getPointerOperand() != &GEP,
>>>> +          "GEP is using the result as the pointer operand!", &GEP);
>>> 
>>> This check is bogus.  The following is legal IR:
>>> 
>>> define void @f() {
>>> ret void
>>> 
>>> notreachedbb:
>>> %xx = getelementptr i8* %xx, i32 0
>>> ret void
>>> }
>>> 
>> I disagree. The code you have there isn't in SSA form, which though there may be holes in the IR format which allows for such non-SSA form, LLVM itself wants to be in SSA form and shouldn't encourage this type of code.
> 
> By "In SSA form", I assume you mean respects the dominance properties
> of SSA.  According to LLVM's definition of dominance, %xx dominates
> %xx.
> 
> Code like that is a natural consequence of allowing unreachable basic
> blocks in IR.
> 
Oh. I didn't know that LLVM defines things in a non-standard way. Using something before it's defined is generally considered bad, if not downright impossible. I therefore maintain that LLVM is wrong, whether it's a "natural consequence" or not (though I have no idea what you mean by it being a "natural consequence").

-bw