[PATCH] D29011: [IR] Add Freeze instruction

Juneyoung Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 5 19:50:41 PST 2019


aqjune added a comment.

In D29011#1733572 <https://reviews.llvm.org/D29011#1733572>, @craig.topper wrote:

> Sorry to come to this late, but does it really make sense for Freeze to be inherited from UnaryOperator? UnaryOperator in my mind is like BinaryOperator which is for arithmetic operations. Freeze is different. The fact that it made sense to add a FreezeOperator hints at that. Should we instead just have a FreezeInst separate from UnaryOperator instead of FreezeOperator?


My understanding of UnaryOperator was that it is a scalar operation that has same input and output type according to the LangRef, so I thought freeze could be categorized into it.
Implementing freeze by inheriting UnaryOperator was straightforward; I did not have to specify instruction's properties, adding it to constexpr was easy, etc. I believe having it inherit UnaryOperator would be good for maintenance, but other than this I have no strong reason for this.
About the FreezeOperator, I think it is natural to have operators in Operator.h that has special purposes: it also has AddOperator / SubOperator / etc defined.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D29011/new/

https://reviews.llvm.org/D29011





More information about the llvm-commits mailing list