Page MenuHomePhabricator

nickdesaulniers (Nick Desaulniers)
Google

Projects

User does not belong to any projects.

User Details

User Since
Apr 13 2018, 4:23 PM (70 w, 3 d)

Recent Activity

Today

nickdesaulniers updated the summary of D66492: [Clang][CodeGen] set alias linkage on QualType.
Tue, Aug 20, 12:54 PM · Restricted Project
nickdesaulniers created D66492: [Clang][CodeGen] set alias linkage on QualType.
Tue, Aug 20, 12:45 PM · Restricted Project
nickdesaulniers requested changes to D66487: Fix -Wimplicit-fallthrough warnings in regcomp.c.
Tue, Aug 20, 11:55 AM · Restricted Project
nickdesaulniers accepted D66487: Fix -Wimplicit-fallthrough warnings in regcomp.c.

Thanks for fixing the build!

Tue, Aug 20, 11:28 AM · Restricted Project

Fri, Aug 16

nickdesaulniers accepted D66341: [AsmPrinter] Remove const qualifier from EmitBasicBlockStart..

Bonus points for *removing* the use of mutable.

Fri, Aug 16, 1:01 PM · Restricted Project

Wed, Aug 14

nickdesaulniers accepted D65019: [ARM] push LR before __gnu_mcount_nc.

Great! LGTM and thank you for this patch. Please give 24hrs for @eli.friedman or @kristof.beyls to leave comments before merging.

Wed, Aug 14, 2:50 PM · Restricted Project, Restricted Project
nickdesaulniers accepted D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467).

LGTM thanks for the patch!

Wed, Aug 14, 10:30 AM · Restricted Project

Tue, Aug 13

nickdesaulniers added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Tue, Aug 13, 1:50 PM · Restricted Project, Restricted Project
nickdesaulniers added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Tue, Aug 13, 1:38 PM · Restricted Project, Restricted Project

Mon, Aug 12

nickdesaulniers added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Mon, Aug 12, 1:30 PM · Restricted Project, Restricted Project

Thu, Aug 8

nickdesaulniers added a comment to D65352: [CodeGen] Require a name for a block addr target.

I think the comment in LLVMTargetMachine::addAsmPrinter about // Don't waste memory on names of temp labels. should be amended in this patch to say ... unless it's address is taken or something to that effect.

Thu, Aug 8, 3:36 PM · Restricted Project
nickdesaulniers added inline comments to D65304: [MC] Don't recreate a label if it's already used.
Thu, Aug 8, 12:02 PM · Restricted Project
nickdesaulniers added inline comments to D65352: [CodeGen] Require a name for a block addr target.
Thu, Aug 8, 11:50 AM · Restricted Project
nickdesaulniers added inline comments to D65352: [CodeGen] Require a name for a block addr target.
Thu, Aug 8, 11:28 AM · Restricted Project
nickdesaulniers added inline comments to D65304: [MC] Don't recreate a label if it's already used.
Thu, Aug 8, 9:39 AM · Restricted Project

Wed, Aug 7

nickdesaulniers added inline comments to D59963: [clang-tidy] Add a module for the Linux kernel..
Wed, Aug 7, 4:02 PM · Restricted Project, Restricted Project, Restricted Project
nickdesaulniers added a comment to D65352: [CodeGen] Require a name for a block addr target.

Creating a name for every block address is a large hammer, but is necessary because at the point when a temp symbol is created we don't necessarily know if it's used in inline asm.

Wed, Aug 7, 3:42 PM · Restricted Project
nickdesaulniers added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Wed, Aug 7, 2:25 PM · Restricted Project, Restricted Project
nickdesaulniers added a reviewer for D65019: [ARM] push LR before __gnu_mcount_nc: nickdesaulniers.
Wed, Aug 7, 2:22 PM · Restricted Project, Restricted Project

Tue, Aug 6

nickdesaulniers added inline comments to D65019: [ARM] push LR before __gnu_mcount_nc.
Tue, Aug 6, 5:06 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D65549: [Sema] Prevent -Wunused-lambda-capture from generating false positive warnings on templated member function.

Thanks for the patch @ziangwan !

Tue, Aug 6, 3:09 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D65019: [ARM] push LR before __gnu_mcount_nc.

Thanks so much for this patch! I look forward to support for __gnu_mcount for the arm32 Linux kernel.

Tue, Aug 6, 2:55 PM · Restricted Project, Restricted Project
nickdesaulniers accepted D65550: [AArch64] Do not emit '#' before immediates in inline asm.
Tue, Aug 6, 2:06 PM · Restricted Project
nickdesaulniers added inline comments to D65550: [AArch64] Do not emit '#' before immediates in inline asm.
Tue, Aug 6, 11:32 AM · Restricted Project

Fri, Jul 26

nickdesaulniers added a comment to D65302: [clang][docs][release notes] mention asm goto support.

Go ahead and commit directly to the branch (I think it can only be done with SVN still), or let me know if you'd like me to do it.

Fri, Jul 26, 2:27 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D65302: [clang][docs][release notes] mention asm goto support.

Thanks for the tips, how does that look @hans ?

Fri, Jul 26, 1:57 PM · Restricted Project, Restricted Project
nickdesaulniers updated the diff for D65302: [clang][docs][release notes] mention asm goto support.
  • add details about ClangBuiltLinux
Fri, Jul 26, 1:57 PM · Restricted Project, Restricted Project

Thu, Jul 25

nickdesaulniers added inline comments to D65302: [clang][docs][release notes] mention asm goto support.
Thu, Jul 25, 3:44 PM · Restricted Project, Restricted Project
nickdesaulniers created D65302: [clang][docs][release notes] mention asm goto support.
Thu, Jul 25, 3:44 PM · Restricted Project, Restricted Project
nickdesaulniers requested changes to D59963: [clang-tidy] Add a module for the Linux kernel..
Thu, Jul 25, 3:33 PM · Restricted Project, Restricted Project, Restricted Project
nickdesaulniers added inline comments to D65184: [Sema] Thread Safety Analysis: Make negative capability typeless..
Thu, Jul 25, 3:27 PM · Restricted Project
nickdesaulniers added inline comments to D65234: [CodeGen]: don't treat structures returned in registers as memory inputs.
Thu, Jul 25, 2:38 PM · Restricted Project
nickdesaulniers added a comment to D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks.

I use svn in-tree, so I tried it out and it does not seem to work for me.

c:\llvm\tools\clang\tools\extra\docs\clang-tidy\checks>python gen-static-analyzer-docs.py
Traceback (most recent call last):
  File "gen-static-analyzer-docs.py", line 120, in <module>
    main()
  File "gen-static-analyzer-docs.py", line 109, in main
    checkers = get_checkers()
  File "gen-static-analyzer-docs.py", line 23, in get_checkers
    result = subprocess.run(["llvm-tblgen", "--dump-json", "-I",
AttributeError: 'module' object has no attribute 'run'

c:\llvm\tools\clang\tools\extra\docs\clang-tidy\checks>python --version
Python 2.7.16
Thu, Jul 25, 10:39 AM · Restricted Project, Restricted Project

Wed, Jul 24

nickdesaulniers accepted D59963: [clang-tidy] Add a module for the Linux kernel..

Re more checks. I guess we can borrow some from the existing checkers:
https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
https://bottest.wiki.kernel.org/coccicheck
https://lwn.net/Articles/752408/
https://lwn.net/Articles/691882/

They do some checks very well. But if we could do something better (more true positives, less false positives), that may be useful.
Also figuring out which of the existing checks in clang-tidy/analyzer are relevant/useful for kernel is another direction.

Wed, Jul 24, 2:22 PM · Restricted Project, Restricted Project, Restricted Project

Tue, Jul 23

nickdesaulniers accepted D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression.

Thanks for the patch and following up on the review.

Tue, Jul 23, 3:31 PM · Restricted Project, Restricted Project

Mon, Jul 22

nickdesaulniers added inline comments to D63889: Check possible warnings on global initializers for reachability.
Mon, Jul 22, 4:23 PM · Restricted Project
nickdesaulniers added inline comments to D63889: Check possible warnings on global initializers for reachability.
Mon, Jul 22, 3:47 PM · Restricted Project
nickdesaulniers added inline comments to D63889: Check possible warnings on global initializers for reachability.
Mon, Jul 22, 3:46 PM · Restricted Project
nickdesaulniers abandoned D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as.
Mon, Jul 22, 12:55 PM · Restricted Project
nickdesaulniers added inline comments to D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling".
Mon, Jul 22, 12:53 PM
nickdesaulniers added inline comments to D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling".
Mon, Jul 22, 12:50 PM

Jul 19 2019

nickdesaulniers added a comment to D64888: Use the MachineBasicBlock symbol for a callbr target.

Reverted in https://reviews.llvm.org/rL366600. While GetBlockAddressSymbol has some complicated machinery related to managing symbols of temporary addresses, I get the feeling a complete fix will lie in modifications there.

Jul 19 2019, 11:26 AM · Restricted Project
nickdesaulniers committed rG4e9196ebcb9d: Revert "Use the MachineBasicBlock symbol for a callbr target" (authored by nickdesaulniers).
Revert "Use the MachineBasicBlock symbol for a callbr target"
Jul 19 2019, 11:20 AM
nickdesaulniers committed rL366600: Revert "Use the MachineBasicBlock symbol for a callbr target".
Revert "Use the MachineBasicBlock symbol for a callbr target"
Jul 19 2019, 11:19 AM
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 19 2019, 10:53 AM · Restricted Project
nickdesaulniers added a comment to D64888: Use the MachineBasicBlock symbol for a callbr target.

For the life of me I swear there's a bug that @craig.topper commented on that explains an issue with symbols not being added to the symbol table at the MC layer, but I cannot find it (and it's killing me). I think that should be pursued (why were we generating a .tmp symbol anyways).

Jul 19 2019, 3:17 AM · Restricted Project
nickdesaulniers added a comment to D64982: [AsmPrinter] Print label if MBB's address is taken.

Sounds like this may address: https://bugs.llvm.org/show_bug.cgi?id=42309#c1?

Jul 19 2019, 3:03 AM · Restricted Project
nickdesaulniers added a comment to D64888: Use the MachineBasicBlock symbol for a callbr target.

It would be nice to merge this fix into 9.0 branch after some time in trunk. What do you think?

@hans @void

Jul 19 2019, 2:54 AM · Restricted Project
nickdesaulniers added inline comments to D64982: [AsmPrinter] Print label if MBB's address is taken.
Jul 19 2019, 2:31 AM · Restricted Project

Jul 18 2019

nickdesaulniers accepted D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 18 2019, 5:54 PM · Restricted Project
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 18 2019, 4:34 PM · Restricted Project

Jul 17 2019

nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 4:48 PM · Restricted Project
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 4:30 PM · Restricted Project
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 4:16 PM · Restricted Project
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 3:24 PM · Restricted Project
nickdesaulniers added inline comments to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 3:15 PM · Restricted Project
nickdesaulniers added a comment to D64888: Use the MachineBasicBlock symbol for a callbr target.
Jul 17 2019, 3:12 PM · Restricted Project

Jul 15 2019

nickdesaulniers added a comment to D64101: [LoopUnroll] fix cloning callbr.

Note to self: this needs to be rebased on top of https://reviews.llvm.org/rL366130 and a test case for LoopUnswitch needs to be added.

Jul 15 2019, 2:21 PM · Restricted Project
nickdesaulniers committed rGc4f245b40aad: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr (authored by nickdesaulniers).
[LoopUnroll+LoopUnswitch] do not transform loops containing callbr
Jul 15 2019, 2:17 PM
nickdesaulniers committed rL366130: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
[LoopUnroll+LoopUnswitch] do not transform loops containing callbr
Jul 15 2019, 2:17 PM
nickdesaulniers closed D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
Jul 15 2019, 2:17 PM · Restricted Project
nickdesaulniers accepted D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss.

I verified the conversion values are correct in the precise warnings in the test cases. From that perspective, and the rest of the code LGTM. Please wait for review from another reviewer. Thanks for the patch.

Jul 15 2019, 10:39 AM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss.

I had duplicated warning for C++11+ - my new warning and C++11’s narrowing warning.

Jul 15 2019, 10:31 AM · Restricted Project, Restricted Project
nickdesaulniers added a reviewer for D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression: nickdesaulniers.
Jul 15 2019, 10:07 AM · Restricted Project, Restricted Project
nickdesaulniers added inline comments to D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression.
Jul 15 2019, 10:07 AM · Restricted Project, Restricted Project

Jul 12 2019

nickdesaulniers added a comment to D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression.

Thanks for the patch. Minor nits looking for test cases that should have no warning (like the two examples provided in the bug).

Jul 12 2019, 4:48 PM · Restricted Project, Restricted Project
nickdesaulniers added inline comments to D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss.
Jul 12 2019, 3:15 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss.

I also recommend re-titling the commit to something like:

[Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss
Jul 12 2019, 2:37 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D64666: [Sema] Enable -Wimplicit-int-float-conversion for integral to floating point precision loss.

Thanks for the patch @ziangwan !

Jul 12 2019, 2:31 PM · Restricted Project, Restricted Project
nickdesaulniers retitled D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as from [Clang][Driver] don't error for unsupported as options for -no-integrates-as to [Clang][Driver] don't error for unsupported as options for -no-integrated-as.
Jul 12 2019, 1:10 PM · Restricted Project
nickdesaulniers accepted D64607: [clang-tidy] Fix crash on end location inside macro.

Thanks for the fix.

Jul 12 2019, 12:26 PM · Restricted Project, Restricted Project
nickdesaulniers added a comment to D64527: driver: Don't warn about assembler flags being unused when not assembling.

Fixup: https://reviews.llvm.org/D64655

Jul 12 2019, 12:04 PM · Restricted Project
nickdesaulniers created D64655: [Clang][Driver] don't error for unsupported as options for -no-integrated-as.
Jul 12 2019, 12:03 PM · Restricted Project

Jul 11 2019

nickdesaulniers added inline comments to D64527: driver: Don't warn about assembler flags being unused when not assembling.
Jul 11 2019, 6:04 PM · Restricted Project
nickdesaulniers added a comment to D64527: driver: Don't warn about assembler flags being unused when not assembling.

ah, maybe the addition of -no-integrated-as shouldn't produce this error?

Jul 11 2019, 5:52 PM · Restricted Project
nickdesaulniers added a comment to D64527: driver: Don't warn about assembler flags being unused when not assembling.

This change breaks building the Linux kernel for arm32 (at least):

Jul 11 2019, 5:49 PM · Restricted Project
nickdesaulniers added a comment to D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.

Thanks for the reviews thus far. Parting thoughts on the newly added test case (llvm/test/Transforms/LoopUnswitch/callbr.ll) before I commit, tomorrow?

Jul 11 2019, 3:23 PM · Restricted Project
nickdesaulniers retitled D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr from [LoopUnroll] do not unroll loops containing callbr to [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
Jul 11 2019, 1:28 PM · Restricted Project
nickdesaulniers updated the diff for D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
  • add test case for LoopUnswitch
Jul 11 2019, 1:28 PM · Restricted Project
nickdesaulniers removed a reviewer for D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr: eli.friedman.
Jul 11 2019, 11:17 AM · Restricted Project

Jul 10 2019

nickdesaulniers added a comment to D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.

it would be good to add a test with callbr there as well, to make sure we do not miss it, in case we remove the restriction again.

Jul 10 2019, 4:55 PM · Restricted Project
nickdesaulniers committed rG8728e4570653: [TargetLowering] support BlockAddress as "i" inline asm constraint (authored by nickdesaulniers).
[TargetLowering] support BlockAddress as "i" inline asm constraint
Jul 10 2019, 10:10 AM
nickdesaulniers committed rL365664: [TargetLowering] support BlockAddress as "i" inline asm constraint.
[TargetLowering] support BlockAddress as "i" inline asm constraint
Jul 10 2019, 10:08 AM
nickdesaulniers closed D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint.
Jul 10 2019, 10:08 AM · Restricted Project
nickdesaulniers removed reviewers for D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint: echristo, void.
Jul 10 2019, 9:58 AM · Restricted Project

Jul 8 2019

nickdesaulniers added a parent revision for D64101: [LoopUnroll] fix cloning callbr: D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint.
Jul 8 2019, 3:02 PM · Restricted Project
nickdesaulniers added a child revision for D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint: D64101: [LoopUnroll] fix cloning callbr.
Jul 8 2019, 3:02 PM · Restricted Project
nickdesaulniers added a parent revision for D64101: [LoopUnroll] fix cloning callbr: D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
Jul 8 2019, 3:02 PM · Restricted Project
nickdesaulniers added a child revision for D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr: D64101: [LoopUnroll] fix cloning callbr.
Jul 8 2019, 3:02 PM · Restricted Project
nickdesaulniers updated the diff for D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
  • add back previous fixes from v2
Jul 8 2019, 2:58 PM · Restricted Project
nickdesaulniers added a comment to D64101: [LoopUnroll] fix cloning callbr.

Marking as requiring changes while the comments are being addressed.

That said, I'd really like to land this patch as it addresses an observable and bad bug in the Linux kernel, and I'd like to do so to make the clang-9 release train, rather than try to rearchitect callbr here.

IIUC the patch as is fixes an issue with unrolling asm goto, in cases unrolling is legal. But we need additional checks to prevent unrolling in the cases Eli mentioned. Maybe a safer approach would be to start with a patch to restrict the unrolling of loops with asm goto (that should also fix the bug in the linux kernel, right?) and then allow unrolling for the safe subset of cases where it is legal.

Jul 8 2019, 2:57 PM · Restricted Project
nickdesaulniers created D64368: [LoopUnroll+LoopUnswitch] do not transform loops containing callbr.
Jul 8 2019, 2:56 PM · Restricted Project
nickdesaulniers added a comment to D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint.

How's that look now, @ostannard ? (Thanks for code review)

Jul 8 2019, 2:03 PM · Restricted Project
nickdesaulniers added a comment to D64101: [LoopUnroll] fix cloning callbr.

Here's another interesting case for me to add a test for: https://godbolt.org/z/WwKXaI

The same address being using in the label list AND explicitly passed in as an address of label. Note the difference in behavior there.

Jul 8 2019, 1:48 PM · Restricted Project
nickdesaulniers added a comment to D64101: [LoopUnroll] fix cloning callbr.

Here's another interesting case for me to add a test for: https://godbolt.org/z/WwKXaI

Jul 8 2019, 1:47 PM · Restricted Project
nickdesaulniers added a comment to D64101: [LoopUnroll] fix cloning callbr.

Or actually, it might make more sense to change the way we generate/lower callbr, to make the label parameters implicit; instead of modeling them with blockaddress, we never write them in IR at all, and automatically generate them later based on the successor list.

That's an appealing idea.

Jul 8 2019, 1:04 PM · Restricted Project
nickdesaulniers updated the diff for D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint.
  • remove missed change
Jul 8 2019, 12:29 PM · Restricted Project
nickdesaulniers updated the diff for D64167: [TargetLowering] support BlockAddress as "i" inline asm constraint.
  • move unit test to new file
Jul 8 2019, 12:28 PM · Restricted Project

Jul 3 2019

nickdesaulniers updated subscribers of D64101: [LoopUnroll] fix cloning callbr.

it needs a "memory" clobber. I think this should be handled like regular inline assembly already in LLVM?

Jul 3 2019, 4:51 PM · Restricted Project
nickdesaulniers added inline comments to D63889: Check possible warnings on global initializers for reachability.
Jul 3 2019, 4:43 PM · Restricted Project