[llvm] r267232 - Revert r267210, it makes clang assert (PR27490).

[Philip Reames via llvm-commits]

Nico,

I have no objection to this being reverted, but in the future, please 
make sure to reply to the original commit thread so that the author 
knows something is going on.  There was a discussion on IRC, but I got 
drug into that quite late and email is our system of record.

Philip

On 04/22/2016 03:08 PM, Nico Weber via llvm-commits wrote:
> Author: nico
> Date: Fri Apr 22 17:08:42 2016
> New Revision: 267232
>
> URL: http://llvm.org/viewvc/llvm-project?rev=267232&view=rev
> Log:
> Revert r267210, it makes clang assert (PR27490).
>
> Modified:
>      llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>      llvm/trunk/test/Transforms/InstCombine/atomic.ll
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=267232&r1=267231&r2=267232&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Fri Apr 22 17:08:42 2016
> @@ -327,8 +327,6 @@ static LoadInst *combineLoadToNewType(In
>     LoadInst *NewLoad = IC.Builder->CreateAlignedLoad(
>         IC.Builder->CreateBitCast(Ptr, NewTy->getPointerTo(AS)),
>         LI.getAlignment(), LI.getName() + Suffix);
> -  NewLoad->setAtomic(LI.getOrdering(), LI.getSynchScope());
> -  assert(!LI.isVolatile() && "volatile unhandled here");
>     MDBuilder MDB(NewLoad->getContext());
>     for (const auto &MDPair : MD) {
>       unsigned ID = MDPair.first;
> @@ -401,8 +399,6 @@ static StoreInst *combineStoreToNewValue
>     StoreInst *NewStore = IC.Builder->CreateAlignedStore(
>         V, IC.Builder->CreateBitCast(Ptr, V->getType()->getPointerTo(AS)),
>         SI.getAlignment());
> -  NewStore->setAtomic(SI.getOrdering(), SI.getSynchScope());
> -  assert(!SI.isVolatile() && "volatile unhandled here");
>     for (const auto &MDPair : MD) {
>       unsigned ID = MDPair.first;
>       MDNode *N = MDPair.second;
> @@ -460,9 +456,9 @@ static StoreInst *combineStoreToNewValue
>   /// later. However, it is risky in case some backend or other part of LLVM is
>   /// relying on the exact type loaded to select appropriate atomic operations.
>   static Instruction *combineLoadToOperationType(InstCombiner &IC, LoadInst &LI) {
> -  // FIXME: We could probably with some care handle both volatile and ordered
> -  // atomic loads here but it isn't clear that this is important.
> -  if (!LI.isUnordered())
> +  // FIXME: We could probably with some care handle both volatile and atomic
> +  // loads here but it isn't clear that this is important.
> +  if (!LI.isSimple())
>       return nullptr;
>   
>     if (LI.use_empty())
> @@ -896,7 +892,6 @@ Instruction *InstCombiner::visitLoadInst
>           V1->setAtomic(LI.getOrdering(), LI.getSynchScope());
>           V2->setAlignment(Align);
>           V2->setAtomic(LI.getOrdering(), LI.getSynchScope());
> -        assert(!LI.isVolatile() && "volatile unhandled here");
>           return SelectInst::Create(SI->getCondition(), V1, V2);
>         }
>   
> @@ -939,9 +934,9 @@ Instruction *InstCombiner::visitLoadInst
>   /// the store instruction as otherwise there is no way to signal whether it was
>   /// combined or not: IC.EraseInstFromFunction returns a null pointer.
>   static bool combineStoreToValueType(InstCombiner &IC, StoreInst &SI) {
> -  // FIXME: We could probably with some care handle both volatile and ordered
> -  // atomic stores here but it isn't clear that this is important.
> -  if (!SI.isUnordered())
> +  // FIXME: We could probably with some care handle both volatile and atomic
> +  // stores here but it isn't clear that this is important.
> +  if (!SI.isSimple())
>       return false;
>   
>     Value *V = SI.getValueOperand();
>
> Modified: llvm/trunk/test/Transforms/InstCombine/atomic.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/atomic.ll?rev=267232&r1=267231&r2=267232&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/InstCombine/atomic.ll (original)
> +++ llvm/trunk/test/Transforms/InstCombine/atomic.ll Fri Apr 22 17:08:42 2016
> @@ -173,45 +173,6 @@ define i32 @test17(i1 %cnd) {
>     ret i32 %x
>   }
>   
> -declare void @clobber()
> -
> -define i32 @test18(float* %p) {
> -; CHECK-LABEL: define i32 @test18(
> -; CHECK: load atomic i32, i32* [[A:%.*]] unordered, align 4
> -; CHECK: store atomic i32 [[B:%.*]], i32* [[C:%.*]] unordered, align 4
> -  %x = load atomic float, float* %p unordered, align 4
> -  call void @clobber() ;; keep the load around
> -  store atomic float %x, float* %p unordered, align 4
> -  ret i32 0
> -}
> -
> -; TODO: probably also legal in this case
> -define i32 @test19(float* %p) {
> -; CHECK-LABEL: define i32 @test19(
> -; CHECK: load atomic float, float* %p seq_cst, align 4
> -; CHECK: store atomic float %x, float* %p seq_cst, align 4
> -  %x = load atomic float, float* %p seq_cst, align 4
> -  call void @clobber() ;; keep the load around
> -  store atomic float %x, float* %p seq_cst, align 4
> -  ret i32 0
> -}
> -
> -define i32 @test20(i32** %p, i8* %v) {
> -; CHECK-LABEL: define i32 @test20(
> -; CHECK: store atomic i8* %v, i8** [[D:%.*]] unordered, align 4
> -  %cast = bitcast i8* %v to i32*
> -  store atomic i32* %cast, i32** %p unordered, align 4
> -  ret i32 0
> -}
> -; TODO: probably also legal in this case
> -define i32 @test21(i32** %p, i8* %v) {
> -; CHECK-LABEL: define i32 @test21(
> -; CHECK: store atomic i32* %cast, i32** %p monotonic, align 4
> -  %cast = bitcast i8* %v to i32*
> -  store atomic i32* %cast, i32** %p monotonic, align 4
> -  ret i32 0
> -}
> -
>   define i32 @test22(i1 %cnd) {
>   ; CHECK-LABEL: define i32 @test22(
>   ; CHECK: [[PHI:%.*]] = phi i32
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits