Page MenuHomePhabricator

belleyb (Benoit Belley)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 6 2015, 9:25 AM (240 w, 4 d)

Recent Activity

Apr 18 2018

belleyb added a comment to D45454: Add llvm_gcov_flush to be called outside a shared library.

@chh I had a chance to try out your proposed changes. It's not causing us any trouble. In fact, __gcov_flush() is not even used at all (at least in LLVM 5.0.1).. I can recompile llvm, compiler_rt and clang and re-run all the tests with __gcov_flush commented out! No problem.

Apr 18 2018, 6:29 AM

Dec 11 2017

belleyb added inline comments to D41081: Fix clang Lexer Windows line-ending bug.
Dec 11 2017, 12:24 PM

Nov 8 2017

belleyb added a comment to D38124: Hide some symbols to avoid a crash on shutdown when using code coverage.

@sylvestre.ledru Thanks for the test!

Nov 8 2017, 6:09 AM · Restricted Project

Nov 2 2017

belleyb added a comment to D38124: Hide some symbols to avoid a crash on shutdown when using code coverage.

Autodesk would also like to see this fix merged into LLVM 5.0.1 and we also agree about the importance of having a regression tests.

Nov 2 2017, 1:12 PM · Restricted Project

Aug 10 2017

belleyb abandoned D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Aug 10 2017, 5:42 AM
belleyb added a comment to D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).

Abandoning this review. The fix has been submitted to the LLVM trunk (6.0 at this time) as r310522. Moreover, we will soon move to LLVM 5.0...

Aug 10 2017, 5:40 AM

Aug 9 2017

belleyb committed rL310522: [Linker] PR33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
[Linker] PR33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals
Aug 9 2017, 1:59 PM
belleyb closed D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals by committing rL310522: [Linker] PR33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Aug 9 2017, 1:59 PM
belleyb committed rL310475: [Support] PR33388 - Fix formatv_object move constructor.
[Support] PR33388 - Fix formatv_object move constructor
Aug 9 2017, 6:48 AM
belleyb closed D34463: bug33388 - Fix formatv_objet copy & move constructors by committing rL310475: [Support] PR33388 - Fix formatv_object move constructor.
Aug 9 2017, 6:47 AM

Jun 28 2017

belleyb added a comment to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

Just to let you know that I've requested commit access.

Jun 28 2017, 3:59 PM

Jun 27 2017

belleyb added a comment to D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).

I will update this code review once https://reviews.llvm.org/D34448 is approved and submitted.

Jun 27 2017, 5:13 PM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 27 2017, 5:11 PM
belleyb updated the diff for D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

Re-organized and simplified test cases to address code review comments.

Jun 27 2017, 5:08 PM
belleyb added a comment to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

Thanks and very thorough testing! I'm not sure all these combinations are needed though, I have some suggestions on paring it down. Once that is done, LGTM. Do you want me to commit for you, or did you want to get commit access?

Jun 27 2017, 10:03 AM
belleyb added inline comments to D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Jun 27 2017, 5:35 AM
belleyb updated the diff for D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).

Converted tests to use lit, llvm-link and FileCheck instead of being written as unit tests

Jun 27 2017, 5:01 AM

Jun 26 2017

belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 26 2017, 5:17 PM
belleyb updated the diff for D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 26 2017, 5:07 PM
belleyb updated the diff for D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

Converted the test cases to Lit + llvm-link + FileCheck (instead of unit tests).

Jun 26 2017, 5:04 PM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 26 2017, 11:49 AM
belleyb added inline comments to D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Jun 26 2017, 9:49 AM
belleyb updated the summary of D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 26 2017, 5:07 AM
belleyb updated the summary of D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Jun 26 2017, 5:07 AM
belleyb updated the summary of D34463: bug33388 - Fix formatv_objet copy & move constructors.
Jun 26 2017, 5:06 AM

Jun 22 2017

belleyb added a comment to D34463: bug33388 - Fix formatv_objet copy & move constructors.

@zturner : Would you mind committing on my behalf ? (I don't have commit access...)

Jun 22 2017, 1:23 PM
belleyb updated the summary of D34463: bug33388 - Fix formatv_objet copy & move constructors.
Jun 22 2017, 1:19 PM
belleyb added inline comments to D34463: bug33388 - Fix formatv_objet copy & move constructors.
Jun 22 2017, 1:18 PM
belleyb updated the diff for D34463: bug33388 - Fix formatv_objet copy & move constructors.

Delete the formatv_object copy constructor as suggested @zturner.

Jun 22 2017, 1:17 PM
belleyb added a comment to D34463: bug33388 - Fix formatv_objet copy & move constructors.

I wonder the exact same thing! We have no use for a copy constructor ourselves. Maybe, it's safer to delete it for now. We can always re-introduce it back if a clear need shows up.

Jun 22 2017, 12:58 PM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 22 2017, 8:17 AM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 22 2017, 7:42 AM

Jun 21 2017

belleyb abandoned D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

Closing. This patch is now irrelevant.

Jun 21 2017, 10:56 AM
belleyb added inline comments to D34463: bug33388 - Fix formatv_objet copy & move constructors.
Jun 21 2017, 10:03 AM
belleyb created D34463: bug33388 - Fix formatv_objet copy & move constructors.
Jun 21 2017, 10:02 AM
belleyb updated the summary of D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Jun 21 2017, 7:36 AM
belleyb updated the summary of D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 21 2017, 7:36 AM
belleyb created D34450: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals (release_40).
Jun 21 2017, 7:26 AM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 21 2017, 7:03 AM
belleyb added a comment to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

I have run the entire LLVM & Clang regression test suite on OS X (both Debug & RelWithDebInfo) to ensure that no regressions were introduced.

Jun 21 2017, 6:51 AM
belleyb added inline comments to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 21 2017, 6:44 AM
belleyb added a comment to D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.

Patch against llvm trunk revision 305867.

Jun 21 2017, 6:32 AM
belleyb created D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals.
Jun 21 2017, 6:28 AM

Jan 29 2015

belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

Thanks Lang, I will investigate some more next week. I'd like to go to the bottom of this issue and understand it thoroughly.

Jan 29 2015, 5:42 PM
belleyb updated subscribers of D7267: Update comments to use unreachable instead of llvm.trap, as implemented now.
Jan 29 2015, 2:30 PM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

Wow! Thanks so much... I am downloading your proposed patch and testing it out inside our product on Windows 7, 8 and 10...

Jan 29 2015, 2:29 PM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

Thanks guys for the comments. I will experiment a little bit more and come back once I have a better understanding. It might also be a difference between the behaviour of the COFF vs ELF object files under Windows.

Jan 29 2015, 1:27 PM
belleyb added a reviewer for D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64: lhames.

Adding Lang to the review so that he can comment on the MCJIT requirements...

Jan 29 2015, 1:08 PM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

majnemer wrote:

I'm a little confused here.

Jan 29 2015, 8:48 AM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

For example, in the x86_64 small code model, one call only use the 32-bit PC relative call to make calls within the same DLL. When calling a function residing in a different DLL, one still has to use a 64-bit safe relocation, which amount to either loading the 64-bit address directly, loading it for a GOT table or jumping to a PLT stub containing the 64-bit call.

Jan 29 2015, 8:15 AM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

AFAIU, the code model isn't imposed by the OS. One can use DLLs compiled with either code model on any versions of Windows. One chooses the code model for each DLL depending on whether he's expecting particular sections (.data and .text) of the DLL to be larger than 2^^24 bytes.

Jan 29 2015, 8:09 AM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

I believe that this should be performed in all memory models.

Jan 29 2015, 6:45 AM
belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

ping

Jan 29 2015, 4:13 AM

Jan 22 2015

belleyb added a comment to D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

Committed as http://llvm.org/viewvc/llvm-project?rev=226830&view=rev

Jan 22 2015, 7:30 AM

Jan 21 2015

belleyb added a comment to D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

No, I don't have direct commit access to the llvm repository. Would "arc land" work once the revision is approved ? (I'll be watching the build bots afterward...)

Jan 21 2015, 6:21 AM
belleyb updated the diff for D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

Added a FIXME notice as suggested by Rafael.

Jan 21 2015, 6:16 AM

Jan 19 2015

belleyb added a comment to D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.

ping

Jan 19 2015, 4:44 AM
belleyb added a comment to D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

ping

Jan 19 2015, 4:44 AM

Jan 15 2015

belleyb added a comment to D6945: Report fatal errors instead of segfaulting/asserting on a few invalid accesses while reading MachO files..

Thanks for taking the time to answer. My comment was by no-mean implying that I was requesting any immediate code change. I completely agree, as I mentioned, in my original comment, that calling report_fatal_error() is infinitely better than crashing on segfault.

Jan 15 2015, 12:23 PM
belleyb added a comment to D6945: Report fatal errors instead of segfaulting/asserting on a few invalid accesses while reading MachO files..

Hi Everyone,

Jan 15 2015, 5:22 AM

Jan 6 2015

belleyb added a comment to D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

Let me know if your agree with the proposed solution or if an other approach would be more appropriate. I am willing to implement any suggested changes,

Jan 6 2015, 11:27 AM
belleyb retitled D6857: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol from to Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.
Jan 6 2015, 11:25 AM
belleyb abandoned D6855: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

I will recreate a new review for this change. It got mixed up with an "arc diff" that was wrongly uploaded. Sorry for the noise. Beginner error...

Jan 6 2015, 11:06 AM
belleyb updated the diff for D6855: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.
Jan 6 2015, 10:50 AM
belleyb updated D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.
Jan 6 2015, 10:41 AM
belleyb retitled D6855: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol from MCJIT/ELF now support MSVC C++ mangled symbol to Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.
Jan 6 2015, 10:40 AM
belleyb updated the test plan for D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.
Jan 6 2015, 10:32 AM
belleyb retitled D6856: Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64 from to Bug 18582 - Offset overflow on calling __chkstk and __alloca on x64.
Jan 6 2015, 10:25 AM
belleyb added a comment to D6855: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol.

Let me know if your agree with the proposed solution or if an other approach would be more appropriate. I am willing to implement any suggested changes,

Jan 6 2015, 9:53 AM
belleyb retitled D6855: Bug 21886 - MCJIT/ELF now support MSVC C++ mangled symbol from to MCJIT/ELF now support MSVC C++ mangled symbol.
Jan 6 2015, 9:37 AM