Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RemoveDIs] Add flag to control loading into new debug mode from bitcode #85649

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Mar 18, 2024

--load-bitcode-into-experimental-debuginfo-iterators

  false: Convert to the old debug mode after reading.
  true: Upgrade to the new debug info format (*).
  unset: Same as false (for now).

(*) As of this patch it actually just means "don't convert to either mode after loading". Auto-upgrading will be implemented in an upcoming patch.

With this flag we can incrementally add support for RemoveDIs by overriding the "unset" behaviour in individual tools. The flag can be removed once all tools support the new debug info mode.

--load-bitcode-into-experimental-debuginfo-iterators

  false: Convert to the old debug mode after reading.
  true: Upgrade to the new debug info format (*).
  unset: Same as false (for now).

(*) As of this patch it actually just means "don't convert to either mode after
loading". Auto-upgrading will be implemented in an upcoming patch.

With this flag we can incrementally add support for RemoveDIs by overriding
the "unset" behaviour in individual tools.
/// support for the RemoveDIs format.
cl::opt<cl::boolOrDefault> LoadBitcodeIntoNewDbgInforFormat(
"load-bitcode-into-experimental-debuginfo-iterators", cl::Hidden);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be to keep consistency with the previous options, include cl::desc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

llvm/lib/Bitcode/Reader/BitcodeReader.cpp Show resolved Hide resolved
llvm/lib/Bitcode/Reader/BitcodeReader.cpp Show resolved Hide resolved
@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 19, 2024

Thanks @CarlosAlbertoEnciso - To answer your two inline questions about the cl::opt being a boolOrDefault: the default value is BOU_UNSET. So at the moment with the check LoadBitcodeIntoNewDbgInforFormat == cl::boolOrDefault::BOU_TRUE in BitcodeReader.cpp, BOU_FALSE and BOU_UNSET have the same meaning. However, we can have individual tools make this change:

if (LoadBitcodeIntoNewDbgInforFormat == cl::boolOrDefault::BOU_UNSET)
    LoadBitcodeIntoNewDbgInforFormat = cl::boolOrDefault::BOU_TRUE;

Which effectively means for that tool only, the default is changed from false to true.

It's useful to be able to give each tool control over this while we're implementing support, so it can be done incrementally. Eventually the flag will be removed.

@CarlosAlbertoEnciso
Copy link
Member

It's useful to be able to give each tool control over this while we're implementing support, so it can be done incrementally. Eventually the flag will be removed.

Thanks for the clarification on cl::opt being a boolOrDefault.

Copy link
Member

@CarlosAlbertoEnciso CarlosAlbertoEnciso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OCHyams OCHyams merged commit 4f909da into llvm:main Mar 19, 2024
3 of 4 checks passed
@OCHyams
Copy link
Contributor Author

OCHyams commented Mar 19, 2024

Thanks both for the review

searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Mar 21, 2024
…ode (llvm#85649)

--load-bitcode-into-experimental-debuginfo-iterators

      false: Convert to the old debug mode after reading.
      true: Upgrade to the new debug info format (*).
      unset: Same as false (for now).

(*) As of this patch it actually just means "don't convert to either
mode after loading". Auto-upgrading will be implemented in an upcoming
patch.

With this flag we can incrementally add support for RemoveDIs by
overriding the "unset" behaviour in individual tools. The flag can be
removed once all tools support the new debug info mode.

Change-Id: I81f7fb83ac95223497d5577354bff7a76b2edb3e
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…ode (llvm#85649)

--load-bitcode-into-experimental-debuginfo-iterators

      false: Convert to the old debug mode after reading.
      true: Upgrade to the new debug info format (*).
      unset: Same as false (for now).

(*) As of this patch it actually just means "don't convert to either
mode after loading". Auto-upgrading will be implemented in an upcoming
patch.

With this flag we can incrementally add support for RemoveDIs by
overriding the "unset" behaviour in individual tools. The flag can be
removed once all tools support the new debug info mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants