[all-commits] [llvm/llvm-project] 1a7042: [mlir] Attempt to resolve edge cases in PassPipeli...

[Christopher Bate via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1a70420ff3b972b3d9bbc1c4d1e98bfa12bfb73a
      https://github.com/llvm/llvm-project/commit/1a70420ff3b972b3d9bbc1c4d1e98bfa12bfb73a
  Author: Christopher Bate <cbate at nvidia.com>
  Date:   2024-12-17 (Tue, 17 Dec 2024)

  Changed paths:
    M mlir/include/mlir/Pass/PassOptions.h
    M mlir/lib/Pass/PassRegistry.cpp
    M mlir/test/Pass/pipeline-options-parsing.mlir

  Log Message:
  -----------
  [mlir] Attempt to resolve edge cases in PassPipeline textual format (#118877)

This commit makes the following changes:

1. Previously certain pipeline options could cause the options parser to
   get stuck in an an infinite loop. An example is:

   ```
mlir-opt %s -verify-each=false
-pass-pipeline='builtin.module(func.func(test-options-super-pass{list={list=1,2},{list=3,4}}))''
   ```

   In this example, the 'list' option of the `test-options-super-pass`
   is itself a pass options specification (this capability was added in
   https://github.com/llvm/llvm-project/issues/101118).

However, while the textual format allows `ListOption<int>` to be given
   as `list=1,2,3`, it did not allow the same format for
   `ListOption<T>` when T is a subclass of `PassOptions` without extra
enclosing `{....}`. Lack of enclosing `{...}` would cause the infinite
   looping in the parser.

   This change resolves the parser bug and also allows omitting the
   outer `{...}` for `ListOption`-of-options.

2. Previously, if you specified a default list value for your
`ListOption`, e.g. `ListOption<int> opt{*this, "list",
llvm::cl::list_init({1,2,3})}`,
it would be impossible to override that default value of `{1,2,3}` with
an *empty* list on the command line, since `my-pass{list=}` was not
allowed.

This was not allowed because of ambiguous handling of lists-of-strings
   (no literal marker is currently required).

   This change makes it explicit in the ListOption construction that we
would like to treat all ListOption as having a default value of "empty"
   unless otherwise specified (e.g. using `llvm::list_init`).

It removes the requirement that lists are not printed if empty. Instead,
   lists are not printed if they do not have their default value.

   It is now clarified that the textual format
   `my-pass{string-list=""}` or `my-pass{string-list={}}`
   is interpreted as "empty list". This makes it imposssible to specify
   that ListOption `string-list` should be a size-1 list containing the
   empty string. However, `my-pass{string-list={"",""}}` *does* specify
a size-2 list containing the empty string. This behavior seems
preferable
   to allow for overriding non-empty defaults as described above.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications