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

Mon Dec 17 05:37:56 PST 2018

javed.absar marked an inline comment as done.
javed.absar added inline comments.

================
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.
----------------
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)

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

https://reviews.llvm.org/D55758