Expression evaluator

[tlemo]

A basic expression evaluator which can be used on top of an fusion IR:

  Fusion fusion;
  FusionGuard fg(&fusion);
  ... create IR ...

  // 1. Create an evaluation context
  EvaluationContext eval_context(&fusion);

  // 2. Bind values
  eval_context.bind(tv0->getRootDomain()->axis(0)->extent(), 100);

  // 3. Evaluate values
  const auto result = ExpressionEvaluator::evaluate(val, eval_context);

[jjsjann123]

LGTM. Please address review comments.

[jjsjann123]

nitpick, not sure if we intended to assert for NDEBUG.
If so we should change this to TORCH_INTERNAL_ASSERT, otherwise, we can move this into the #else block above to avoid confusion.

[csarofeen]

Can we do an explicit RTTI check to make sure the cast is valid?

[tlemo]

@jjsjann123 : I moved the check inside the #else path and replaced it with TORCH_INTERNAL_ASSERT. There's a small problem the TORCH_INTERNAL_ASSERT (and TORCK_CHECK) - they report failures as exceptions. Which means that it's problematic to have anything which uses them in destructors.

Can we do an explicit RTTI check to make sure the cast is valid?

Yes, that's the dynamic_cast<> path in debug builds (no check in release builds since this is a replacement for static_cast)

[jjsjann123]

Neg mapping to ! looks dangerous.
Can we have add a test with Neg on integer as well as bool to confirm the behavior?

[tlemo]

Hmm, I don't know what was I thinking (probably read Not instead of Neg). Fixed.

[jjsjann123]

looks like LT and And is not getting tested.

[tlemo]

Good catch. I meant to remove the support for LT since we only support integral results.

[csarofeen]

Only thing that concerns me is as() everything else looks good to me.

[csarofeen]

We could enforce that the evaluation is only valid after lowering the code. We could do this by running through all input TensorViews and making sure their domain extents are either constant or a named scalar.

[tlemo]

Why? What's wrong with evaluating expressions before lowering?

[csarofeen]

Really excited about the implications of being able to do this :-)

[csarofeen]

Thanks for the clean up

[csarofeen]

Can we do an explicit RTTI check to make sure the cast is valid?

[csarofeen]

nit: noexcept?

[tlemo]

As an optimization it's not helping much (most every C++ compiler would infer this from the definition, which is even inline here)

I'm a bit concerned about adding noexcept everywhere since it can actually add an overhead (compiler must generate code to check & enforce it unless it can prove it's not needed).

I can add it if you insist, although it creates a slippery slope: in the absence of a detailed set of guidelines, the most it's used the more it encourages overuse, which in turn creates more problems than it solves (in particular, the unfortunate choice to have TORCH_CHECK report failures as exceptions makes it hard to predict where exceptions can originate from a cursory review)

TL;DR: noexcept is a interface contract specification, not an optimization hint.

[jjsjann123]

My requests are addressed.