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

[CIR][CIRGen] Handle __extension__ keyword #421

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Conversation

YazZz1k
Copy link
Contributor

@YazZz1k YazZz1k commented Jan 24, 2024

Support __extension__ keyword in CIRGen

@YazZz1k
Copy link
Contributor Author

YazZz1k commented Jan 24, 2024

The repro-case

int a(void) { return __extension__ 0b101010; }

Compilation of this test fails with

NYI
UNREACHABLE executed at /home/huawei/cir/repo/van/llvm-project/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp:560!

Copy link

github-actions bot commented Jan 24, 2024

✅ With the latest revision this PR passed the C/C code formatter.

@YazZz1k YazZz1k force-pushed the cpp-127 branch 2 times, most recently from 5bbc8ec to c6e4682 Compare January 24, 2024 12:49
mlir::Value VisitUnaryExtension(const UnaryOperator *E) {
llvm_unreachable("NYI");
TestAndClearIgnoreResultAssign();
Copy link
Member

Choose a reason for hiding this comment

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

This diverges from the original clang CodeGen. Could you add some explanation regarding this change?

Copy link
Contributor Author

@YazZz1k YazZz1k Jan 26, 2024

Choose a reason for hiding this comment

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

I can't), because it is my mistake. I fixed this

@bcardosolopes
Copy link
Member

@lanza just rebased and this has the side effect of breaking GH PR workflow, making it impossible to review, apologies. Please rebase!

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Thanks, mostly good! Some inline comments.

int foo(void) { return __extension__ 0b101010; }

//CHECK: cir.func @foo() -> !s32i extra( {inline = #cir.inline<no>, optnone = #cir.optnone} ) {
//CHECK-NEXT: %0 = cir.alloca !s32i, cir.ptr <!s32i>, ["__retval"] {alignment = 4 : i64}
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpicks here:

  • The file name is misspelled, should be gnu-extension.
  • Use FileCheck regexes for MLIR values, for example:
%[[addr:.*]] = cir.alloca !s32i, cir.ptr <!s32i>,
%[[const:.*]] = cir.const(#cir.int<42> : !s32i) : !s32i
cir.store %[[const]], %[[addr]] : !s32i, cir.ptr <!s32i>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, got it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM

@bcardosolopes bcardosolopes merged commit f308835 into llvm:main Jan 31, 2024
5 of 6 checks passed
lanza pushed a commit that referenced this pull request Mar 23, 2024
Support \_\_extension\_\_ keyword in CIRGen
eZWALT pushed a commit to eZWALT/clangir that referenced this pull request Mar 24, 2024
Support \_\_extension\_\_ keyword in CIRGen
lanza pushed a commit that referenced this pull request Apr 29, 2024
Support \_\_extension\_\_ keyword in CIRGen
lanza pushed a commit that referenced this pull request Apr 29, 2024
Support \_\_extension\_\_ keyword in CIRGen
eZWALT pushed a commit to eZWALT/clangir that referenced this pull request Apr 29, 2024
Support \_\_extension\_\_ keyword in CIRGen
lanza pushed a commit that referenced this pull request Apr 29, 2024
Support \_\_extension\_\_ keyword in CIRGen
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.

3 participants