Migrate IntImm & FloatImm ObjectRef to not-null #5788
Conversation
| @@ -123,7 +123,7 @@ PrimExpr RewriteSimplifier::Impl::VisitExpr_(const AddNode* op) { | |||
| // Pattern var to match any expression | |||
| PVar<PrimExpr> x, y, z, b1, b2, s1, s2; | |||
| // Pattern var match IntImm | |||
| PVar<IntImm> c1, c2, c3; | |||
| PVarOpt<Optional<IntImm>> c1, c2, c3; | |||
There was a problem hiding this comment.
I don't think it is right to use optional here, as the intention is to match the variable itself
There was a problem hiding this comment.
Yes, I agree. It is only to accommodate absent of default constructor. However the behaviour don't violate with PVar I think.
There was a problem hiding this comment.
I agree too...Is there any better to do this?
There was a problem hiding this comment.
I have 2 possible approach here:
A0: Rename PVarOpt --> PVarExt(Extended) and hide Optional input format.
Look like --> PVarExt<IntImm> c1, c2, c3;
A1: Provide default constructor with default values.
Look like: PVar<IntImm> c1(IntImm(DataType::Int(32), 0)), c2(IntImm(DataType::Int(32), 0)), c3(IntImm(DataType::Int(32), 0));
Please let me know your thoughts on this. Thanks!
There was a problem hiding this comment.
I don't think either approaches are good here. Let us leave it as it is for now
Refer #5318
@tqchen, @jroesch, @zhiics, @junrushao1994 : Please help review, Thanks!