[llvm-commits] [llvm] r96282 - in /llvm/trunk: lib/VMCore/Verifier.cpp unittests/VMCore/VerifierTest.cpp
Chris Lattner
clattner at apple.com
Mon Feb 15 22:36:23 PST 2010
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
>>
More information about the llvm-commits
mailing list