[llvm-bugs] [Bug 50822] New: FIROps.td needs a cleanup/upgrade

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=50822

            Bug ID: 50822
           Summary: FIROps.td needs a cleanup/upgrade
           Product: flang
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Fortran IR
          Assignee: unassignedbugs at nondot.org
          Reporter: joker.eph at gmail.com
                CC: David.Truby at arm.com, eschweitz at nvidia.com,
                    jperier at nvidia.com, kirankumartp at gmail.com,
                    llvm-bugs at lists.llvm.org

https://reviews.llvm.org/D104167 broke the flang build, as I was trying to fix
it I was patching through FIROps.td and found it difficult to work with.

First there is a lot of C++ inline there. We should keep inline C++ limited to
method declaration and short accessor (a couple lines of C++). The rest can be
outlined in FIROps.cpp : this is quite important for anyone using an IDE.

Another problem here is that methods that ends in `AttrName` are usually
generated by TableGen. Here is an example where a manually defined method on
the class conflict with ODS:
https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2331

In general there seems to be many cases where there is redundancy between
manually defined C++ method and what ODS already generates, or would generate
if the attributes are defined in the `let arguments` list.
Some ops like `fir_ConstcOp` does not even declare its arguments right now:
https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2835

Some cases of manually written verifier are actually redundant with the
verification emitted by ODS already, like here:
https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L282-L283
; this verifier is actually duplicated across other ops and would benefit just
be a type constraint that would be on the results declaration to avoid written
a verifier at all.


Many of these could also use the declarative assembly syntax and avoid C++
parsers/printers in the first place.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210623/92d9b3f5/attachment-0001.html>