[PATCH] D55758: [TableGen] : Extend !if semantics through new language feature !ifs

[Javed Absar via Phabricator via llvm-commits]

javed.absar marked 2 inline comments as done.
javed.absar added a comment.

Thanks Hal.

So ``!cond(condition1 : val1, condition2 : val2, ..., default : valn)``

  Instead of embedding !if inside !if which can get cumbersome,
  one can use !ifs. !ifs returns 'val1' if the result of 'int' or 'bit'
  operator 'condition1' is nonzero. Otherwise, it checks 'condition2'.
  If 'condition2' is nonzero, returns 'val2', and so on.
  If all conditions are zero, the default value 'valn' is returned.



================
Comment at: docs/TableGen/LangIntro.rst:261-266
+``!ifs(condition1 : val1, condition2 : val2, ..., default : valn)``
+    Instead of embedding !if inside !if which can get cumbersome,
+    one can use !ifs. !ifs returns 'val1' if the result of 'int' or 'bit'
+    operator 'condition1' is nonzero. Otherwise, it checks 'condition2'.
+    If 'condition2' is nonzero, returns 'val2', and so on.
+    If all conditions are zero, the default value 'valn' is returned.
----------------
hfinkel wrote:
> javed.absar wrote:
> > kristof.beyls wrote:
> > > Right now, it seems "!ifs" is somewhat a mix of some syntactic sugar over nested ifs and a switch statement.
> > > The syntax of the example below makes me instinctively read this as something akin a switch statement.
> > > However, the semantics described above indicates clearly that order matters: the first nonzero condition matters.
> > > If you want the logic of nested ifs, maybe it's easier to read to the non-expert Tablegen reader (which I guess is the majority of people reading Tablegen files) if the syntax still reflected nested ifs?
> > > 
> > > I wonder if it wouldn't be better overall to move to calling this "switch" rather than "ifs", but then also make it an error for more than 1 condition to be non-zero.
> > > That way, the order wouldn't matter.
> > > Would that actually be useful?
> > I am fine with calling it 'switch' if that is more intuitive to the users. Making it an error for more than one condition to be true - I am not sure if users will like that restriction.
> > 
> > Other option could be to call it !cond( (test1, action1), (test2, action2)...) much like LISP cond - https://www.tutorialspoint.com/lisp/lisp_cond_construct.htm . It is an error if none of the conditions are true, and the 'default' can achieved by ending always with a true clause i.e. (1, actionN)
> > 
> > 
> FWIW, I like '!cond' better than '!ifs', although I like your original syntax matter than the more LISP-like syntax. Making it an error if none of the conditions are true, and there is no default, makes sense to me. I'd restrict default to appear last, unless there's some reason to do otherwise.
Thanks Hal for the suggestion.
I will rework this patch to take following approach:

'!cond' '(' [ Value : Val ]+, DEFAULT: Value ')'

The default must be present and should be last.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55758