Re: [llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

2008-02-15 Thread Chris Lattner 
On Feb 15, 2008, at 11:41 AM, Dale Johannesen wrote:
 I think this makes isPaddingElement and a bunch of code in llvm-
 types.cpp dead.

 Yes, thought I'd see how this patch went first:)

Heh ok :)  Looks like you were right :)

 Doesn't this also cause us to regress on PR1278?

 If that's what you want to call it.  Personally I think padding should
 be
 deterministic whenever possible, even though the standards don't
 require it; you get fewer Interesting bugs that way.

Ok, but this still doesn't get us that, and I don't see how we  
reasonably could ever get it in general:

struct mystruct x;
x.f = 1;
x.g = 2;

leaves any padding uninitialized.

-Chris
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

Re: [llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

2008-02-15 Thread Dale Johannesen 

On Feb 15, 2008, at 11:56 AM, Chris Lattner wrote:

 On Feb 15, 2008, at 11:41 AM, Dale Johannesen wrote:
 I think this makes isPaddingElement and a bunch of code in llvm-
 types.cpp dead.

 Yes, thought I'd see how this patch went first:)

 Heh ok :)  Looks like you were right :)

 Doesn't this also cause us to regress on PR1278?

 If that's what you want to call it.  Personally I think padding  
 should
 be
 deterministic whenever possible, even though the standards don't
 require it; you get fewer Interesting bugs that way.

 Ok, but this still doesn't get us that, and I don't see how we
 reasonably could ever get it in general:

 struct mystruct x;
 x.f = 1;
 x.g = 2;

 leaves any padding uninitialized.

You can do it by initializing all struct objects, say to 0.  The  
overhead for that is probably too high; in practice there would be  
cases you wouldn't get.

___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

Re: [llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

2008-02-15 Thread Dale Johannesen 

On Feb 15, 2008, at 11:38 AM, Chris Lattner wrote:


 On Feb 13, 2008, at 10:36 AM, Dale Johannesen wrote:

 Author: johannes
 Date: Wed Feb 13 12:36:09 2008
 New Revision: 47077

 URL: http://llvm.org/viewvc/llvm-project?rev=47077view=rev
 Log:
 Don't omit copying of PaddingElements; this causes
 wrong code when structs are identical except that
 one has padding in the same place another has a
 real field.  Look at the right node when looking
 for MODIFY under RET.

 I think this makes isPaddingElement and a bunch of code in llvm-
 types.cpp dead.

Yes, thought I'd see how this patch went first:)

 Doesn't this also cause us to regress on PR1278?

If that's what you want to call it.  Personally I think padding should  
be
deterministic whenever possible, even though the standards don't
require it; you get fewer Interesting bugs that way.

 I'm fine with temporarily saying we'll take a code pessimization for
 improved correctness but we need to address this somehow, perhaps by
 filing another bug.

 -Chris





 Modified:
   llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

 Modified: llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp?rev=47077r1=47076r2=47077view=diff

 =
 =
 =
 =
 =
 =
 =
 =
 = 
 =
 --- llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp (original)
 +++ llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp Wed Feb 13 12:36:09 2008
 @@ -1246,8 +1246,6 @@
const StructLayout *SL = getTargetData().getStructLayout(STy);
Constant *Zero = ConstantInt::get(Type::Int32Ty, 0);
for (unsigned i = 0, e = STy-getNumElements(); i != e; ++i) {
 -  if (isPaddingElement(STy, i))
 -continue;
  Constant *Idx = ConstantInt::get(Type::Int32Ty, i);
  Value *Idxs[2] = { Zero, Idx };
  Value *DElPtr = Builder.CreateGEP(DestLoc.Ptr, Idxs, Idxs + 2,
 tmp);
 @@ -1721,7 +1719,8 @@
// operand is an aggregate value, create a temporary to evaluate
 it into.
MemRef DestLoc;
const Type *DestTy = ConvertType(TREE_TYPE(TREE_OPERAND(exp, 0)));
 -if (!DestTy-isFirstClassType()  TREE_CODE(exp) !=  
 MODIFY_EXPR)
 +if (!DestTy-isFirstClassType() 
 +TREE_CODE(TREE_OPERAND(exp, 0)) != MODIFY_EXPR)
  DestLoc = CreateTempLoc(DestTy);
Emit(TREE_OPERAND(exp, 0), DestLoc.Ptr ? DestLoc : NULL);
  }


 ___
 llvm-commits mailing list
 llvm-commits@cs.uiuc.edu
 http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

 ___
 llvm-commits mailing list
 llvm-commits@cs.uiuc.edu
 http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

Re: [llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

2008-02-15 Thread Chris Lattner 

On Feb 13, 2008, at 10:36 AM, Dale Johannesen wrote:

 Author: johannes
 Date: Wed Feb 13 12:36:09 2008
 New Revision: 47077

 URL: http://llvm.org/viewvc/llvm-project?rev=47077view=rev
 Log:
 Don't omit copying of PaddingElements; this causes
 wrong code when structs are identical except that
 one has padding in the same place another has a
 real field.  Look at the right node when looking
 for MODIFY under RET.

I think this makes isPaddingElement and a bunch of code in llvm- 
types.cpp dead.  Doesn't this also cause us to regress on PR1278?

I'm fine with temporarily saying we'll take a code pessimization for  
improved correctness but we need to address this somehow, perhaps by  
filing another bug.

-Chris





 Modified:
llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

 Modified: llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
 URL: 
 http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp?rev=47077r1=47076r2=47077view=diff

 = 
 = 
 = 
 = 
 = 
 = 
 = 
 = 
 ==
 --- llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp (original)
 +++ llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp Wed Feb 13 12:36:09 2008
 @@ -1246,8 +1246,6 @@
 const StructLayout *SL = getTargetData().getStructLayout(STy);
 Constant *Zero = ConstantInt::get(Type::Int32Ty, 0);
 for (unsigned i = 0, e = STy-getNumElements(); i != e; ++i) {
 -  if (isPaddingElement(STy, i))
 -continue;
   Constant *Idx = ConstantInt::get(Type::Int32Ty, i);
   Value *Idxs[2] = { Zero, Idx };
   Value *DElPtr = Builder.CreateGEP(DestLoc.Ptr, Idxs, Idxs + 2,  
 tmp);
 @@ -1721,7 +1719,8 @@
 // operand is an aggregate value, create a temporary to evaluate  
 it into.
 MemRef DestLoc;
 const Type *DestTy = ConvertType(TREE_TYPE(TREE_OPERAND(exp, 0)));
 -if (!DestTy-isFirstClassType()  TREE_CODE(exp) != MODIFY_EXPR)
 +if (!DestTy-isFirstClassType() 
 +TREE_CODE(TREE_OPERAND(exp, 0)) != MODIFY_EXPR)
   DestLoc = CreateTempLoc(DestTy);
 Emit(TREE_OPERAND(exp, 0), DestLoc.Ptr ? DestLoc : NULL);
   }


 ___
 llvm-commits mailing list
 llvm-commits@cs.uiuc.edu
 http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

[llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

2008-02-13 Thread Dale Johannesen 
Author: johannes
Date: Wed Feb 13 12:36:09 2008
New Revision: 47077

URL: http://llvm.org/viewvc/llvm-project?rev=47077view=rev
Log:
Don't omit copying of PaddingElements; this causes
wrong code when structs are identical except that
one has padding in the same place another has a
real field.  Look at the right node when looking
for MODIFY under RET.


Modified:
llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

Modified: llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
URL: 
http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp?rev=47077r1=47076r2=47077view=diff

==
--- llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp (original)
+++ llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp Wed Feb 13 12:36:09 2008
@@ -1246,8 +1246,6 @@
 const StructLayout *SL = getTargetData().getStructLayout(STy);
 Constant *Zero = ConstantInt::get(Type::Int32Ty, 0);
 for (unsigned i = 0, e = STy-getNumElements(); i != e; ++i) {
-  if (isPaddingElement(STy, i))
-continue;
   Constant *Idx = ConstantInt::get(Type::Int32Ty, i);
   Value *Idxs[2] = { Zero, Idx };
   Value *DElPtr = Builder.CreateGEP(DestLoc.Ptr, Idxs, Idxs + 2, tmp);
@@ -1721,7 +1719,8 @@
 // operand is an aggregate value, create a temporary to evaluate it into.
 MemRef DestLoc;
 const Type *DestTy = ConvertType(TREE_TYPE(TREE_OPERAND(exp, 0)));
-if (!DestTy-isFirstClassType()  TREE_CODE(exp) != MODIFY_EXPR)
+if (!DestTy-isFirstClassType()  
+TREE_CODE(TREE_OPERAND(exp, 0)) != MODIFY_EXPR)
   DestLoc = CreateTempLoc(DestTy);
 Emit(TREE_OPERAND(exp, 0), DestLoc.Ptr ? DestLoc : NULL);
   }


___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits