[PATCH] D24577: [RFC] Move synchronization scope and atomic orderings from SDNode to MachineMemOperand

[Justin Lebar via llvm-commits]

jlebar added a comment.

In https://reviews.llvm.org/D24577#548365, @kzhuravl wrote:

> Currently machine memory operand's member layout has a 4 byte hole between `BaseAlignLog2` and `AAInfo`.


Only on 64-bit platforms, right?  Are we willing to bite a 4-byte increase to MMO size on 32-bit?



> MachineMemOperand.h:131
> +    /// Reserved/unused.
> +    unsigned Reserved : 23;
> +  };

I don't think we need this field.

> MachineMemOperand.h:151
> +    assert(getFailureOrdering() == FailureOrdering && "Value truncated");
> +  }
> +

Can we do this inside the constructor?

> SelectionDAGNodes.h:1198
> +    return MMO->getSuccessOrdering();
>    }
>  

Should we make the MMO and AtomicSDNode interfaces match?  Right now we have getOrdering and getSuccessOrdering on MMO, but only getOrdering on the SDNode.

I think I'd be OK having two functions that both return the success ordering if we assert inside getFailureOrdering / getSuccessOrdering that we have a cmpxchg instruction.  Otherwise maybe having only the one function makes sense.

https://reviews.llvm.org/D24577