-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb] Add RISCV for Makefile.rules #124758
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
[lldb] Add RISCV for Makefile.rules #124758
Conversation
|
@llvm/pr-subscribers-lldb Author: None (kper) ChangesAs discussed with @DavidSpickett in discord. Running the test runner caused the following issue: By overriding the flags, we avoid the Full diff: https://github.com/llvm/llvm-project/pull/124758.diff 1 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 2da6ff226b615c..06959f226066ac 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -207,6 +207,10 @@ else
override ARCH :=
override ARCHFLAG :=
endif
+ ifeq "$(ARCH)" "riscv"
+ override ARCH :=
+ override ARCHFLAG :=
+ endif
ifeq "$(findstring mips,$(ARCH))" "mips"
override ARCHFLAG := -
endif
|
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
My understanding of this is it generates -m64 and -m32 flags for architectures that need it, and RISC-V is not one of those.
(I have heard of toolchains with both 32 and 64 bit but they use a different triple for each iirc)
|
@kper Please let us know if you need one of us to press the merge button for you. |
@JDevlieghere Thank you, I think that one of you needs to merge it. Next to this message is also a "Update branch" button but I am not sure whether I will lose the approval if I rebase. |
GitHub seems to let you do almost anything without losing existing approvals, for better or worse :) It would have been fine anyway, it's just doing the rebase onto main that "squash and merge" will do anyway. |
|
Please set a valid email address first - https://llvm.org/docs/DeveloperPolicy.html#github-email-address Then this can be merged. |
As discussed with @DavidSpickett in discord. Running the test runner caused the following issue:
By overriding the flags, we avoid the
-mriscv.