User Details
- User Since
- Mar 6 2013, 6:15 AM (533 w, 3 d)
May 10 2022
LGTM
May 9 2022
May 7 2022
LGTM. Thanks.
LGTM. Thanks.
Apr 28 2022
It looks like this resolves PR/53722 not 53772.
LGTM
Mar 7 2022
Mar 3 2022
Do you have commit access?
Feb 23 2022
Feb 22 2022
Jan 20 2022
LGTM with a small fix. I will commit the changes shortly.
Jan 11 2022
Dec 31 2021
LGTM. Thanks for the patch.
Dec 24 2021
Thanks for the patch. Some notes are below.
Dec 14 2021
Thanks for review.
Dec 13 2021
- Reduce redundancy of the test case
- Add IsMips64 field to the Object class.
- Save reference to an Object in the RelocationSection class.
Since the "Mips64EL" property is a property of the input object, I wonder if we should just make it a member of the object,
and then put a reference to that object in the relocation sections, so that we can query that, rather than needing
to a) pass around the variable, and b) have to have the property in the relocation section? What do you think?
Dec 5 2021
LGTM with some nits.
Oct 28 2021
LGTM
Oct 21 2021
The patch committed at rG302a165
Do you have commit access?
- Does this problem exist on all versions of Debian or starting from specific version only?
- This fix needs test cases. Take a look at "Check linker invocation on Debian 6 MIPS 32/64-bit" in the clang/test/Driver/linux-ld.c for example.
Oct 18 2021
LGTM. But get one more acceptance from any other reviewer before commit.
LGTM. Thanks for the fix.
Oct 12 2021
Thanks for the patch.
LGTM. Do you have commit access?
- This patch needs test cases. Take a look at llvm/unittests/ADT/TripleTest.cpp
- IMHO the current fix is not the best solution. It solves the problem but looks a bit strange. For example, the following code (unusual but correct) stops working. Without the patch it produces a correct "arm-linux-gnu" target triple.
Triple T("mipsisa32r6-linux-gnu"); T.setArch(Triple::arm);
It's better to modify Triple::setArch() function so it takes two arguments ArchType and SubArchType. The second argument is NoSubARch by default. Then the setArch() passes both arguments to the getArchTypeName(). We need to teach this function to generate architecture's name in accordance with ArchType and SubArchType arguments. In this patch we can do it for MIPS only. @MaskRay - what do you think?
Sep 27 2021
If we construct T = Triple("mipsisa64r6el-unknown-linux-gnuabi64") what is result of the T.getArchName() call (without your patch)? If getArchName() returns mipsisa64r6el we need to investigate what is the reason for redundant setArch() call somewhere in the clang driver code. If getArchName() does not return mipsisa64r6el we need to fix triple parsing. One more question - why other targets which has "sub architectures" does not need to extend the Triple::setArch method?
Sep 24 2021
LGTM
Sep 15 2021
Sep 14 2021
Aug 19 2021
For compatibility with tools unsupported 64-bit pc-relative relocations the patch introduces new command line options in Clang: mmips-pc64-rel and mno-mips-pc64-rel. These options passed to LLVM by Clang driver as -mmips-pc64-rel={true|false}. I could not find any better way to pass the option into MC layer.
Aug 8 2021
Thanks for review.
- Check section name prefix .debug_ instead of section type. This approach is closer to other MIPS toolchains.
Aug 7 2021
Thanks for review.
Aug 6 2021
Jul 12 2021
LGTM
Jul 10 2021
LGTM
May 5 2021
LGTM
The MIPS changes LGTM
May 3 2021
LGTM
Apr 27 2021
LGTM
LGTM
Apr 26 2021
Apr 21 2021
LGTM
Apr 7 2021
AFAIK the MIPS LLVM-based toolchain mentioned in D13340 have not been implemented.
Mar 17 2021
Mar 11 2021
LGTM
Mar 10 2021
LGTM
Mar 9 2021
I see. Thanks. One more question - is it possible to split the patch: one for "fixing a couple of rounding issues" and another for "Introducing a few more conversion rules"?
I have not checked the patch carefully yet. But it looks like some tests was removed. For example MIPS64R5EL-LABEL: i8_2... Is it intended?
Feb 2 2021
Jan 18 2021
@ntesic Do you have commit access?
Jan 14 2021
LGTM
Nov 25 2020
LGTM
Nov 13 2020
Nov 4 2020
My bad, I missed that you implemented passing the option to backend.
Nov 2 2020
What's a goal of this change? Do you want to suppress an error message when the option provided to Clang? If so, is it a real-life case?
Oct 30 2020
Oct 28 2020
LGTM with a few nits. Thanks.
Oct 12 2020
MIPS changes LGTM
Oct 9 2020
LGTM
Oct 6 2020
LGTM. Thanks!
Sep 29 2020
LGTM
Sep 25 2020
LGTM
Sep 24 2020
LGTM
Sep 20 2020
LGTM
Aug 17 2020
LGTM