[llvm-commits] [llvm] r96282 - in /llvm/trunk: lib/VMCore/Verifier.cpp unittests/VMCore/VerifierTest.cpp

[Chris Lattner]

On Feb 15, 2010, at 10:24 PM, Chandler Carruth wrote:

> On Mon, Feb 15, 2010 at 4:42 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
>> Chris Lattner wrote:
>>> Isn't a unit test for this completely overkill?
>> 
>> Testing methodology-type people would tell you that the real problem is
>> that the rest of the verifier is lacking tests ...
>> 
>> ... but really, writing this test case is what led to the fixes in
>> r96272, r96273 and r96275. Even if we didn't test this in particular, we
>> should probably have a unit test that F->dump() and FPM.run(F) on
>> functions that have no module owners still work.
>> 
>> Ultimately I don't really care.
> 
> FWIW, I like it. The cost is so low that even just verifying that
> someone hasn't typoed something egregiously seems to justify it. But
> I'm one of those testing methodology types... ;]

I don't like it (if it creeps beyond this) because it adds more uses of the C++ API, meaning that the tests need to be updated when the API changes.  Why not use the stable C api for stuff like this?

-Chris

> 
>> 
>> Nick
>> 
>>> 
>>> -Chris
>>> 
>>> On Feb 15, 2010, at 2:09 PM, Nick Lewycky wrote:
>>> 
>>>> Author: nicholas
>>>> Date: Mon Feb 15 16:09:09 2010
>>>> New Revision: 96282
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=96282&view=rev
>>>> Log:
>>>> Teach the verifier to check the condition on a branch and ensure that it has
>>>> 'i1' type.
>>>> 
>>>> Added:
>>>>     llvm/trunk/unittests/VMCore/VerifierTest.cpp
>>>> 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=96282&r1=96281&r2=96282&view=diff
>>>> 
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/VMCore/Verifier.cpp (original)
>>>> +++ llvm/trunk/lib/VMCore/Verifier.cpp Mon Feb 15 16:09:09 2010
>>>> @@ -317,6 +317,7 @@
>>>>      void visitStoreInst(StoreInst&SI);
>>>>      void visitInstruction(Instruction&I);
>>>>      void visitTerminatorInst(TerminatorInst&I);
>>>> +    void visitBranchInst(BranchInst&BI);
>>>>      void visitReturnInst(ReturnInst&RI);
>>>>      void visitSwitchInst(SwitchInst&SI);
>>>>      void visitSelectInst(SelectInst&SI);
>>>> @@ -749,6 +750,14 @@
>>>>    visitInstruction(I);
>>>> }
>>>> 
>>>> +void Verifier::visitBranchInst(BranchInst&BI) {
>>>> +  if (BI.isConditional()) {
>>>> +    Assert2(BI.getCondition()->getType()->isIntegerTy(1),
>>>> +            "Branch condition is not 'i1' type!",&BI, BI.getCondition());
>>>> +  }
>>>> +  visitTerminatorInst(BI);
>>>> +}
>>>> +
>>>> void Verifier::visitReturnInst(ReturnInst&RI) {
>>>>    Function *F = RI.getParent()->getParent();
>>>>    unsigned N = RI.getNumOperands();
>>>> 
>>>> Added: llvm/trunk/unittests/VMCore/VerifierTest.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/VMCore/VerifierTest.cpp?rev=96282&view=auto
>>>> 
>>>> ==============================================================================
>>>> --- llvm/trunk/unittests/VMCore/VerifierTest.cpp (added)
>>>> +++ llvm/trunk/unittests/VMCore/VerifierTest.cpp Mon Feb 15 16:09:09 2010
>>>> @@ -0,0 +1,44 @@
>>>> +//===- llvm/unittest/VMCore/VerifierTest.cpp - Verifier unit tests --------===//
>>>> +//
>>>> +//                     The LLVM Compiler Infrastructure
>>>> +//
>>>> +// This file is distributed under the University of Illinois Open Source
>>>> +// License. See LICENSE.TXT for details.
>>>> +//
>>>> +//===----------------------------------------------------------------------===//
>>>> +
>>>> +#include "llvm/Constants.h"
>>>> +#include "llvm/DerivedTypes.h"
>>>> +#include "llvm/Function.h"
>>>> +#include "llvm/Instructions.h"
>>>> +#include "llvm/LLVMContext.h"
>>>> +#include "llvm/Analysis/Verifier.h"
>>>> +#include "gtest/gtest.h"
>>>> +
>>>> +namespace llvm {
>>>> +namespace {
>>>> +
>>>> +TEST(VerifierTest, Branch_i1) {
>>>> +  LLVMContext&C = getGlobalContext();
>>>> +  FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), /*isVarArg=*/false);
>>>> +  Function *F = Function::Create(FTy, GlobalValue::ExternalLinkage);
>>>> +  BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
>>>> +  BasicBlock *Exit = BasicBlock::Create(C, "exit", F);
>>>> +  ReturnInst::Create(C, Exit);
>>>> +
>>>> +  // To avoid triggering an assertion in BranchInst::Create, we first create
>>>> +  // a branch with an 'i1' condition ...
>>>> +
>>>> +  Constant *False = ConstantInt::getFalse(C);
>>>> +  BranchInst *BI = BranchInst::Create(Exit, Exit, False, Entry);
>>>> +
>>>> +  // ... then use setOperand to redirect it to a value of different type.
>>>> +
>>>> +  Constant *Zero32 = ConstantInt::get(IntegerType::get(C, 32), 0);
>>>> +  BI->setOperand(0, Zero32);
>>>> +
>>>> +  EXPECT_TRUE(verifyFunction(*F, ReturnStatusAction));
>>>> +}
>>>> +
>>>> +}
>>>> +}
>>>> 
>>>> 
>>>> _______________________________________________
>>>> 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
>>