[llvm-commits] atomics memoperands patch

[Evan Cheng]

On Jun 20, 2008, at 9:40 AM, Mon P Wang wrote:

>
>
>
> On Jun 20, 2008, at 12:28 AM, Evan Cheng wrote:
>
>> Questions for everyone. I've just noticed something in
>> SelectionDAG.cpp:
>>
>> SDNode *SelectionDAG::FindModifiedNodeSlot(SDNode *N,
>>                                           SDOperandPtr Ops,unsigned
>> NumOps,
>>                                           void *&InsertPos) {
>> ...
>>
>>  FoldingSetNodeID ID;
>>  AddNodeIDNode(ID, N->getOpcode(), N->getVTList(), Ops, NumOps);
>>
>>  if (const LoadSDNode *LD = dyn_cast<LoadSDNode>(N)) {
>>    ID.AddInteger(LD->getAddressingMode());
>>    ID.AddInteger(LD->getExtensionType());
>>    ID.AddInteger(LD->getMemoryVT().getRawBits());
>>    ID.AddInteger(LD->getAlignment());
>>    ID.AddInteger(LD->isVolatile());
>>  } else if (const StoreSDNode *ST = dyn_cast<StoreSDNode>(N)) {
>>    ID.AddInteger(ST->getAddressingMode());
>>    ID.AddInteger(ST->isTruncatingStore());
>>    ID.AddInteger(ST->getMemoryVT().getRawBits());
>>    ID.AddInteger(ST->getAlignment());
>>    ID.AddInteger(ST->isVolatile());
>>  }
>>
>> Do we need the if () else if? AddNodeIDNode has already added those
>> fields to the FoldingSetNodeId.
>>
>
> Looking at the code, it does seem wrong to add the fields twice.

I think so too. Chris?

>
>
>> Also, do we want to cse nodes that are volatile?
>>
>
> If the CSE can prove that no needed side effects are produced, it can
> do so.  In general though, I would shy away from doing so as it might
> not be easy to prove that is the case.

Right.

Evan

>
>
>  -- Mon Ping
>
>> Evan
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits