[llvm-commits] [llvm] r147861 - in /llvm/trunk: include/llvm/Bitcode/BitCodes.h include/llvm/Target/TargetLowering.h lib/AsmParser/LLParser.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/CodeGen/GCMetadata.cpp lib/CodeGen/LLVMTargetMachine.cpp lib/
David Blaikie
dblaikie at gmail.com
Wed Jan 11 06:20:21 PST 2012
On Wed, Jan 11, 2012 at 2:25 AM, Duncan Sands <baldrick at free.fr> wrote:
> Hi Chandler,
>
>> Add 'llvm_unreachable' to passify GCC's understanding of the constraints
>> of several newly un-defaulted switches. This also helps optimizers
>> (including LLVM's) recognize that every case is covered, and we should
>> assume as much.
I have another question, though - did this actually improve LLVM's
codegen? If so, should Clang be inserting the unreachable instructions
automatically based on the reachability analysis it does?
>
> previously if you added a new value to one of these enums then you would get a
> compiler warning from clang, while now you now longer get a compiler warning
> (right?). In short you changed from detecting a problem at compile time to
> detecting a problem at run time... Not good!
That's not quite what he fixed.
-Wswitch-enum fires if a switch over an enum doesn't cover all of the
enum values (& doesn't have a default case). My change removed
defaults that weren't necessary (because all cases were already
covered) which is a common feedback on CRs so that we can utilize this
warning effectively (I have an outstanding patch for clang to add the
warning I used to find these cases so we don't have to catch it
manually in CR).
The fallout from this was that GCC's (but not Clang's) -Wreturn-type
warning, which, among other things, checks that you have a return
statement on all paths out of a function with a return type specified,
fires unnecessarily because it doesn't understand that all the paths
are handled by the switch. Take this example:
enum foo { a, b };
foo func();
int func2() {
switch (func()) {
case a:
return 0;
case b:
return 1;
}
}
GCC: "labels.cpp:10:1: warning: control reaches end of non-void
function [-Wreturn-type]"
Clang: Silent
So, to fix GCC's lack of insight, Chandler added llvm_unreachable
/after/ the switch so that GCC stopped complaining about the missing
return there. This doesn't stop -Wswitch-enum from firing under either
GCC or Clang because -Wswitch-enum doesn't care about returns or
reachability, only about whether every enum value is handled in a
switch over an enum.
Hopefully that makes sense,
- David
>
> Ciao, Duncan.
>
>>
>> Modified:
>> llvm/trunk/include/llvm/Bitcode/BitCodes.h
>> llvm/trunk/include/llvm/Target/TargetLowering.h
>> llvm/trunk/lib/AsmParser/LLParser.cpp
>> llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>> llvm/trunk/lib/CodeGen/GCMetadata.cpp
>> llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp
>> llvm/trunk/lib/CodeGen/MachineInstr.cpp
>> llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
>> llvm/trunk/lib/CodeGen/Spiller.cpp
>> llvm/trunk/lib/MC/MCExpr.cpp
>> llvm/trunk/lib/Support/APFloat.cpp
>> llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> llvm/trunk/lib/VMCore/Verifier.cpp
>>
>> Modified: llvm/trunk/include/llvm/Bitcode/BitCodes.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/BitCodes.h?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Bitcode/BitCodes.h (original)
>> +++ llvm/trunk/include/llvm/Bitcode/BitCodes.h Tue Jan 10 12:08:01 2012
>> @@ -20,6 +20,7 @@
>>
>> #include "llvm/ADT/SmallVector.h"
>> #include "llvm/Support/DataTypes.h"
>> +#include "llvm/Support/ErrorHandling.h"
>> #include<cassert>
>>
>> namespace llvm {
>> @@ -122,6 +123,7 @@
>> case Blob:
>> return false;
>> }
>> + llvm_unreachable("Invalid encoding");
>> }
>>
>> /// isChar6 - Return true if this character is legal in the Char6 encoding.
>>
>> Modified: llvm/trunk/include/llvm/Target/TargetLowering.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetLowering.h?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Target/TargetLowering.h (original)
>> +++ llvm/trunk/include/llvm/Target/TargetLowering.h Tue Jan 10 12:08:01 2012
>> @@ -117,6 +117,7 @@
>> // Extend by copying the sign bit.
>> return ISD::SIGN_EXTEND;
>> }
>> + llvm_unreachable("Invalid content kind");
>> }
>>
>> /// NOTE: The constructor takes ownership of TLOF.
>>
>> Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
>> +++ llvm/trunk/lib/AsmParser/LLParser.cpp Tue Jan 10 12:08:01 2012
>> @@ -2549,6 +2549,7 @@
>> return Error(ID.Loc, "constant expression type mismatch");
>> return false;
>> }
>> + llvm_unreachable("Invalid ValID");
>> }
>>
>> bool LLParser::ParseValue(Type *Ty, Value *&V, PerFunctionState *PFS) {
>>
>> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
>> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Tue Jan 10 12:08:01 2012
>> @@ -134,6 +134,7 @@
>> case AcquireRelease: return bitc::ORDERING_ACQREL;
>> case SequentiallyConsistent: return bitc::ORDERING_SEQCST;
>> }
>> + llvm_unreachable("Invalid ordering");
>> }
>>
>> static unsigned GetEncodedSynchScope(SynchronizationScope SynchScope) {
>> @@ -141,6 +142,7 @@
>> case SingleThread: return bitc::SYNCHSCOPE_SINGLETHREAD;
>> case CrossThread: return bitc::SYNCHSCOPE_CROSSTHREAD;
>> }
>> + llvm_unreachable("Invalid synch scope");
>> }
>>
>> static void WriteStringRecord(unsigned Code, StringRef Str,
>> @@ -372,6 +374,7 @@
>> case GlobalValue::LinkerPrivateWeakLinkage: return 14;
>> case GlobalValue::LinkerPrivateWeakDefAutoLinkage: return 15;
>> }
>> + llvm_unreachable("Invalid linkage");
>> }
>>
>> static unsigned getEncodedVisibility(const GlobalValue *GV) {
>> @@ -380,6 +383,7 @@
>> case GlobalValue::HiddenVisibility: return 1;
>> case GlobalValue::ProtectedVisibility: return 2;
>> }
>> + llvm_unreachable("Invalid visibility");
>> }
>>
>> // Emit top-level description of module, including target triple, inline asm,
>>
>> Modified: llvm/trunk/lib/CodeGen/GCMetadata.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GCMetadata.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/GCMetadata.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/GCMetadata.cpp Tue Jan 10 12:08:01 2012
>> @@ -148,6 +148,7 @@
>> case GC::PreCall: return "pre-call";
>> case GC::PostCall: return "post-call";
>> }
>> + llvm_unreachable("Invalid point kind");
>> }
>>
>> bool Printer::runOnFunction(Function&F) {
>>
>> Modified: llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp Tue Jan 10 12:08:01 2012
>> @@ -96,6 +96,7 @@
>> case cl::BOU_TRUE: return true;
>> case cl::BOU_FALSE: return false;
>> }
>> + llvm_unreachable("Invalid verbose asm state");
>> }
>>
>> // Enable or disable FastISel. Both options are needed, because
>>
>> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Tue Jan 10 12:08:01 2012
>> @@ -221,6 +221,7 @@
>> case MachineOperand::MO_Metadata:
>> return getMetadata() == Other.getMetadata();
>> }
>> + llvm_unreachable("Invalid machine operand type");
>> }
>>
>> /// print - Print the specified machine operand.
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Tue Jan 10 12:08:01 2012
>> @@ -3016,6 +3016,7 @@
>> case TargetLowering::C_Memory:
>> return 3;
>> }
>> + llvm_unreachable("Invalid constraint type");
>> }
>>
>> /// Examine constraint type and operand type and determine a weight value.
>>
>> Modified: llvm/trunk/lib/CodeGen/Spiller.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/Spiller.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/Spiller.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/Spiller.cpp Tue Jan 10 12:08:01 2012
>> @@ -194,4 +194,5 @@
>> case trivial: return new TrivialSpiller(pass, mf, vrm);
>> case inline_: return createInlineSpiller(pass, mf, vrm);
>> }
>> + llvm_unreachable("Invalid spiller optimization");
>> }
>>
>> Modified: llvm/trunk/lib/MC/MCExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCExpr.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/MC/MCExpr.cpp (original)
>> +++ llvm/trunk/lib/MC/MCExpr.cpp Tue Jan 10 12:08:01 2012
>> @@ -17,6 +17,7 @@
>> #include "llvm/MC/MCSymbol.h"
>> #include "llvm/MC/MCValue.h"
>> #include "llvm/Support/Debug.h"
>> +#include "llvm/Support/ErrorHandling.h"
>> #include "llvm/Support/raw_ostream.h"
>> using namespace llvm;
>>
>> @@ -216,6 +217,7 @@
>> case VK_Mips_GOT_PAGE: return "GOT_PAGE";
>> case VK_Mips_GOT_OFST: return "GOT_OFST";
>> }
>> + llvm_unreachable("Invalid variant kind");
>> }
>>
>> MCSymbolRefExpr::VariantKind
>>
>> Modified: llvm/trunk/lib/Support/APFloat.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Support/APFloat.cpp (original)
>> +++ llvm/trunk/lib/Support/APFloat.cpp Tue Jan 10 12:08:01 2012
>> @@ -1172,6 +1172,7 @@
>> case rmTowardNegative:
>> return sign == true;
>> }
>> + llvm_unreachable("Invalid rounding mode found");
>> }
>>
>> APFloat::opStatus
>>
>> Modified: llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp (original)
>> +++ llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp Tue Jan 10 12:08:01 2012
>> @@ -240,6 +240,7 @@
>> case Constant::GlobalRelocations:
>> return SectionKind::getDataRel();
>> }
>> + llvm_unreachable("Invalid relocation");
>> }
>>
>> /// SectionForGlobal - This method computes the appropriate section to emit
>>
>> Modified: llvm/trunk/lib/VMCore/Verifier.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Verifier.cpp?rev=147861&r1=147860&r2=147861&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/VMCore/Verifier.cpp (original)
>> +++ llvm/trunk/lib/VMCore/Verifier.cpp Tue Jan 10 12:08:01 2012
>> @@ -238,6 +238,7 @@
>> MessagesStr<< "compilation terminated.\n";
>> return true;
>> }
>> + llvm_unreachable("Invalid action");
>> }
>>
>>
>>
>>
>> _______________________________________________
>> 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