[llvm-commits] [llvm] r159876 - in /llvm/trunk: lib/Transforms/InstCombine/InstructionCombining.cpp test/Transforms/InstCombine/badmalloc.ll test/Transforms/InstCombine/invoke.ll test/Transforms/InstCombine/malloc-free-delete.ll test/Transforms/InstCom

[John Criswell]

On 7/9/12 11:00 AM, Duncan Sands wrote:
> Hi Nuno,
>
> On 08/07/12 22:01, Nuno Lopes wrote:
>>>> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Fri Jul  6
>>>> 18:09:25 2012
>>>> @@ -1137,12 +1137,29 @@
>>>>          }
>>>>        }
>>>>        if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(U)) {
>>>> -      if (II->getIntrinsicID() == Intrinsic::lifetime_start ||
>>>> -          II->getIntrinsicID() == Intrinsic::lifetime_end) {
>>>> +      switch (II->getIntrinsicID()) {
>>>> +      default: return false;
>>>> +      case Intrinsic::memmove:
>>>> +      case Intrinsic::memcpy:
>>>> +      case Intrinsic::memset: {
>>>> +        MemIntrinsic *MI = cast<MemIntrinsic>(II);
>>>> +        if (MI->isVolatile() || MI->getRawDest() != V)
>>> why exclude volatile stores?  If all that is being done to the allocated memory
>>> is doing a bunch of volatile stores to it, I don't see why you can't discard
>>> them and the allocation too.
>> I thought about that, but.. I guess it's better not to mess around with
>> volatiles. And I don't even think it's legal (C standard wise) to remove it.
> we already remove volatile stores to alloca's, so why not to heap allocations
> too?

IMHO, it is incorrect to optimize away volatile stores of any kind. The 
whole point of volatile is to tell the compiler that something weird is 
going on that it doesn't understand and that it should leave the memory 
access alone.  I sometimes use (at the C language level) a volatile 
variable to ensure that a load or store doesn't get optimized away 
(e.g., when writing tests for SAFECode, I want to write simple tests, 
but I need the loads and stores to stay in place so that SAFECode 
instruments them).

That said, I understand that people disagree with me on that issue, but 
if you do, you should fix the Language Reference Manual to indicate 
which volatiles can be removed and which ones cannot.

-- John T.


>
> Ciao, Duncan.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits