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

[Support] Remove redundant forwarding functions read/write (NFC) #66051

Merged

Conversation

kazutakahirata
Copy link
Contributor

We don't need these forwarding functions if we add a default template
parameter to their callees.

We don't need these forwarding functions if we add a default template
parameter to their callees.
Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

Change looks good to me.

@ldionne - any idea about the premerge check failure? I can't seem to make sense of it - is it some flake/infrastructure failure, or is it suggesting some actual problem with this patch?

@kazutakahirata
Copy link
Contributor Author

Thanks for reviewing the patch!

@ldionne - any idea about the premerge check failure? I can't seem to make sense of it - is it some flake/infrastructure failure, or is it suggesting some actual problem with this patch?

It looks like the premerge check is run immediately after a pull request is created, and it never gets updated.

@kazutakahirata kazutakahirata merged commit 9f7906a into llvm:main Sep 17, 2023
1 of 2 checks passed
@kazutakahirata kazutakahirata deleted the pr_support_endian_unaligned branch September 17, 2023 07:03
@ldionne
Copy link
Member

ldionne commented Sep 18, 2023

Change looks good to me.

@ldionne - any idea about the premerge check failure? I can't seem to make sense of it - is it some flake/infrastructure failure, or is it suggesting some actual problem with this patch?

It definitely looks infrastructure-related to me, not an issue with this patch. I'm trying to figure out whether that was a flake here: #66650. Thanks for the heads up.

@ldionne
Copy link
Member

ldionne commented Sep 18, 2023

Yeah, it looks like a flake because that other PR successfully generated the CI pipeline.

@dwblaikie
Copy link
Collaborator

Yeah, it looks like a flake because that other PR successfully generated the CI pipeline.

Thanks for taking a look - anything we can do to look into the flake in more detail? Be great to improve the reliability of this sort of infrastructure.

@ldionne
Copy link
Member

ldionne commented Sep 18, 2023

Honestly this would be a question for @metaflow , since the buildkite pipeline generation runs on the service machines, which he's in charge of.

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…m#66051)

We don't need these forwarding functions if we add a default template
parameter to their callees.
@metaflow
Copy link
Contributor

looking on output of pipeline script - it was based on a old revision ci that had a bug. Rebasing to a never revision should have fixed that

zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…m#66051)

We don't need these forwarding functions if we add a default template
parameter to their callees.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…m#66051)

We don't need these forwarding functions if we add a default template
parameter to their callees.
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

4 participants