<html>
<head>
<base href="https://bugs.llvm.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - FIROps.td needs a cleanup/upgrade"
href="https://bugs.llvm.org/show_bug.cgi?id=50822">50822</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>FIROps.td needs a cleanup/upgrade
</td>
</tr>
<tr>
<th>Product</th>
<td>flang
</td>
</tr>
<tr>
<th>Version</th>
<td>trunk
</td>
</tr>
<tr>
<th>Hardware</th>
<td>PC
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>enhancement
</td>
</tr>
<tr>
<th>Priority</th>
<td>P
</td>
</tr>
<tr>
<th>Component</th>
<td>Fortran IR
</td>
</tr>
<tr>
<th>Assignee</th>
<td>unassignedbugs@nondot.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>joker.eph@gmail.com
</td>
</tr>
<tr>
<th>CC</th>
<td>David.Truby@arm.com, eschweitz@nvidia.com, jperier@nvidia.com, kirankumartp@gmail.com, llvm-bugs@lists.llvm.org
</td>
</tr></table>
<p>
<div>
<pre><a href="https://reviews.llvm.org/D104167">https://reviews.llvm.org/D104167</a> 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:
<a href="https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2331">https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2331</a>
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:
<a href="https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2835">https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L2835</a>
Some cases of manually written verifier are actually redundant with the
verification emitted by ODS already, like here:
<a href="https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L282-L283">https://github.com/llvm/llvm-project/blob/ec08f03be3942d4ae6694d0f7a9b490fe3cbba9b/flang/include/flang/Optimizer/Dialect/FIROps.td#L282-L283</a>
; 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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>