This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] Add true and false literals to represent booleans
ClosedPublic

Authored by Paul-C-Anagnostopoulos on Nov 2 2020, 11:27 AM.

Details

Summary

This revision adds the 'true' and 'false' literals to TableGen, which represent boolean values.

This is primarily syntactic sugar aimed at improving the readability of TableGen files. Once parsed, these literals are converted to 1 and 0. Note that they can be used in bit sequences, too.

The documentation was updated. A test was added and a couple of other tests updated due to an improved error message.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2020, 11:27 AM
Paul-C-Anagnostopoulos requested review of this revision.Nov 2 2020, 11:28 AM
llvm/lib/TableGen/TGLexer.cpp
344

I will remove this old code.

387

I will remove this old code.

I cleaned out the obsolete code lines.

I updated one more test to use the new literals.

nhaehnle accepted this revision.Nov 4 2020, 8:57 AM

LGTM

llvm/lib/TableGen/TGParser.cpp
2247

Unrelated change.

This revision is now accepted and ready to land.Nov 4 2020, 8:57 AM
jrtc27 added a subscriber: jrtc27.Nov 5 2020, 12:46 PM

Should these not be bits<1> instead? C/C++'s lax rules around integer promotion aren't generally something modern languages replicate.

Sorry, I'm not sure what you mean by "these." If you mean 'true' and 'false', they are simply named literals for 1 and 0. So they can be used in any context where an integer is allowed. TableGen already promotes bit and bit<n> to int and vice versa automatically.

A field type of bit is used for boolean fields. It is also used for single-bit fields where the individual bits feature of bits<n> is not required.

I contemplated inventing a boolean type, but quickly gave up on that idea.

Sorry, I'm not sure what you mean by "these." If you mean 'true' and 'false', they are simply named literals for 1 and 0. So they can be used in any context where an integer is allowed.

A field type of 'bit' is used for boolean fields. It is also used for single-bit fields where the individual bits feature of bits<n> is not required.

I contemplated inventing a 'boolean' type, but quickly gave up on that idea.

Yes, I meant the literals. It's less than ideal IMO that bits<3> = true is valid; I personally think that should be a type error.

I would have to invent a boolean type to prevent that assignment. That would be such a box of frogs that I quickly gave up on the idea. I'm not even sure we could do it compatibly. What would !eq(x, y) produce? And I don't think people would want !if(pred, ...) to require a boolean, since it doesn't in C++. [Much to my chagrin, and perhaps yours.]

Now, I might be able to make a special case out of field and let assignments. But do we want true/false suddenly to become "sort of a typed literal"?

My heart is with you, but I'm not sure we can do it.

jrtc27 added a comment.Nov 5 2020, 1:06 PM

I would have to invent a boolean type to prevent that assignment. That would be such a box of frogs that I quickly gave up on the idea. I'm not even sure we could do it compatibly. What would !eq(x, y) produce? And I don't think people would want !if(pred, ...) to require a boolean, since it doesn't in C++. [Much to my chagrin, and perhaps yours.]

Now, I might be able to make a special case out of field and let assignments. But do we want true/false suddenly to become "sort of a typed literal"?

My heart is with you, but I'm not sure we could do it.

Hm, if those already produce an integer rather than bit/bits<1> then yeah it seems our hands are tied here without disruptive changes.

Yes, I think it would be quite disruptive. But a strong stylistic suggestion in the documentation is in order.

Note that I have already modified one TableGen file to this:

bits<3> foo = {true, false, true};

Because the bits field is actually treated as three booleans. Consider status registers and predicate registers, too.