[llvm] r213197 - trivial fix for PR20314

[Duncan P. N. Exon Smith]

> On 2014-Jul-16, at 14:08, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> Author: spatel
> Date: Wed Jul 16 16:08:10 2014
> New Revision: 213197
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=213197&view=rev
> Log:
> trivial fix for PR20314
> 
> Make sure that the AddrInst is an Instruction.
> 

Thanks for fixing this!

> Added:
>    llvm/trunk/test/CodeGen/X86/pr20314.ll
> Modified:
>    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=213197&r1=213196&r2=213197&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
> +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Wed Jul 16 16:08:10 2014
> @@ -2130,8 +2130,11 @@ bool AddressingModeMatcher::MatchOperati
>     return true;
>   }
>   case Instruction::SExt: {
> +    // Make sure this isn't a ConstantExpr (PR20314).

We don't usually put bug numbers in the comments.  Please remove.

> +    Instruction *SExt = dyn_cast<Instruction>(AddrInst);
> +    if (!SExt) return false;

It's not in the coding standards, but clang-format prefers this return on
the next line (I do too).

> +
>     // Try to move this sext out of the way of the addressing mode.
> -    Instruction *SExt = cast<Instruction>(AddrInst);
>     // Ask for a method for doing so.
>     TypePromotionHelper::Action TPH = TypePromotionHelper::getAction(
>         SExt, InsertedTruncs, TLI, PromotedInsts);
> 
> Added: llvm/trunk/test/CodeGen/X86/pr20314.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr20314.ll?rev=213197&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/pr20314.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/pr20314.ll Wed Jul 16 16:08:10 2014

It'd be better to find a name that describes the situation.  Something
like `address-type-promotion-constantexpr.ll`.

> @@ -0,0 +1,14 @@
> +; RUN: llc < %s -mtriple=x86_64-pc-linux
> +
> +; No check as PR20314 is a crashing bug.

In general, it's best to add CHECK lines even for crashers if there's
anything reasonable to check for.  Not sure what you'd add here, but
think about whether there's something reasonable.

BTW, listing the PR at the top here (as you've done) *is* a good idea.

> +
> + at c = common global [2 x i32] zeroinitializer, align 4
> + at a = common global i32 0, align 4
> + at b = internal unnamed_addr constant [2 x i8] c"\01\00", align 1
> +
> +define i32 @main() {
> +entry:
> +  %foo = load i8* getelementptr ([2 x i8]* @b, i64 0, i64 sext (i8 or (i8 zext (i1 icmp eq (i32* getelementptr inbounds ([2 x i32]* @c, i64 0, i64 1), i32* @a) to i8), i8 1) to i64)), align 1
> +  ret i32 0
> +}
> +
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits