[PATCH] D113665: [flang] Add a test case for `fir.dt_entry`

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski abandoned this revision.
awarzynski added a comment.

Abandon in favor of https://reviews.llvm.org/D113674.



================
Comment at: flang/test/Fir/convert-to-llvm-invalid.fir:37
+// expected-error at +1{{failed to legalize operation 'fir.dt_entry'}}
+fir.dt_entry "method", @method_impl
----------------
clementval wrote:
> kiranchandramohan wrote:
> > I think @clementval was suggesting that dt_entry should always. come inside a dispatch_table. The fact that dt_entry can be written in module scope shows that it is missing some constraints.
> > 
> > I think dt_entry operation should have the HasParent constraint where the parent is the dispatch_table.
> > 
> > https://mlir.llvm.org/docs/Traits/#hasparent
> > https://github.com/llvm/llvm-project/blob/9534e361ea12aaecde52b8ac4c947f9a301d0c9b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td#L155
> > 
> > I was thinking that the test was skipped because if it has the constraint maybe we never reach the conversion of dt_entry and hence the error is never produced.
> Cannot summarized it better than @kiranchandramohan. Doesn't really makes sense to have a `fir.dt_entry` alone in the wild. 
> I was thinking that the test was skipped because if it has the constraint maybe we never reach the conversion of dt_entry and hence the error is never produced.

The conversion *is hit*, so clearly something is missing somewhere :) I added this test so have something that documents the current behavior. If that conversion is never hit (nor it should be), then perhaps it ought to be deleted?

Anyway, thanks for the context and for clarifying! Let's try ttps://reviews.llvm.org/D113674 instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113665