Page MenuHomePhabricator

gchatelet (Guillaume Chatelet)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 7 2017, 7:28 AM (79 w, 2 d)

Recent Activity

Mon, Nov 26

gchatelet committed rCTE347570: [clang-tidy] Improving narrowing conversions.
[clang-tidy] Improving narrowing conversions
Mon, Nov 26, 8:29 AM
gchatelet committed rL347570: [clang-tidy] Improving narrowing conversions.
[clang-tidy] Improving narrowing conversions
Mon, Nov 26, 8:28 AM
gchatelet closed D53488: [clang-tidy] Improving narrowing conversions.
Mon, Nov 26, 8:28 AM · Restricted Project
gchatelet accepted D54895: [llvm-exegesis] [NFC] Fixing typo..
Mon, Nov 26, 6:26 AM
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

LGTM!

Mon, Nov 26, 5:38 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Removed redundant options in regression tests
Mon, Nov 26, 5:22 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Fixing documentation.
Mon, Nov 26, 1:21 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Thank you for bearing with me @aaron.ballman.

Mon, Nov 26, 1:06 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressing comments
Mon, Nov 26, 1:06 AM · Restricted Project

Fri, Nov 23

gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Fri, Nov 23, 1:15 PM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Added a new option warn about wrong type literals
Fri, Nov 23, 1:11 PM · Restricted Project

Thu, Nov 22

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Reverting the WarnOnFloatingPointNarrowingConversion option to be on by default
Thu, Nov 22, 3:33 AM · Restricted Project
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Thu, Nov 22, 2:58 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Refactored the code to make it simpler, added more tests
Thu, Nov 22, 2:47 AM · Restricted Project

Tue, Nov 20

gchatelet accepted D54343: [llvm-exegesis][NFC] Some code style cleanup.
Tue, Nov 20, 1:19 AM

Fri, Nov 16

gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Fri, Nov 16, 8:08 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressed comments
Fri, Nov 16, 8:08 AM · Restricted Project

Nov 14 2018

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • State char signess clearly
Nov 14 2018, 8:07 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Thx for the review. I have two suggestions in the comments let me know what you think.

Nov 14 2018, 8:07 AM · Restricted Project

Nov 13 2018

gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

So, finally LGTM :)
Please give @aaron.ballman a chance to comment, as he reviewed too.

Thanks for your patch!

Nov 13 2018, 7:16 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Remove debugging leftover
Nov 13 2018, 6:57 AM · Restricted Project
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Nov 13 2018, 5:40 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Address comments
Nov 13 2018, 5:39 AM · Restricted Project

Nov 12 2018

gchatelet added a comment to D54383: [llvm-exegesis] Analysis: writeMeasurementValue(): don't alloc string for double each time..

I'm not entirely convinced here:

  • The code is harder to read,
  • the formatting pattern and the SmallString size may go out of sync,
  • the performance win is not that big.
Nov 12 2018, 2:29 AM
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Nov 12 2018, 2:00 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Address comments + fix hex values display
Nov 12 2018, 2:00 AM · Restricted Project

Nov 11 2018

gchatelet added inline comments to D54343: [llvm-exegesis][NFC] Some code style cleanup.
Nov 11 2018, 11:39 PM

Nov 9 2018

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressing comments, adding Option to disable FP to FP narrowing warnings, handling FP literals.
Nov 9 2018, 8:32 AM · Restricted Project
gchatelet accepted D54304: [llvm-exegesis][NFC] More tests for ExegesisTarget::fillMemoryOperands()..

Thx!

Nov 9 2018, 6:17 AM
gchatelet accepted D54297: [llvm-exegesis][NFC] Add a way to declare the default counter binding for unbound CPUs for a target..
Nov 9 2018, 2:37 AM

Nov 8 2018

gchatelet retitled D53488: [clang-tidy] Improving narrowing conversions from [clang-tidy] Catching narrowing from double to float. to [clang-tidy] Improving narrowing conversions.
Nov 8 2018, 8:37 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Is this good enough to go? @JonasToth

Nov 8 2018, 8:29 AM · Restricted Project
gchatelet accepted D54252: [llvm-exegesis][NFC] Add missing header guard + cosmetics..
Nov 8 2018, 2:59 AM

Nov 7 2018

gchatelet accepted D54151: [llvm-exegesis] Increasing wrapping limit..
Nov 7 2018, 1:28 AM
gchatelet added a comment to D54185: [PowerPC][llvm-exegesis] Add a PowerPC target.

Nice ! Thx for the patch !

Nov 7 2018, 12:54 AM
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressing comments
Nov 7 2018, 12:50 AM · Restricted Project
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Nov 7 2018, 12:50 AM · Restricted Project

Nov 6 2018

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressing comments and enhancing diagnostics
Nov 6 2018, 9:21 AM · Restricted Project
gchatelet accepted D54144: [llvm-exegesis] Correclty handle all X86 memory encoding formats..
Nov 6 2018, 9:12 AM
gchatelet added a comment to D54151: [llvm-exegesis] Increasing wrapping limit..

Did you run the test suite? We may have wrapping lines in the tests.

Nov 6 2018, 9:11 AM
gchatelet accepted D54147: [llvm-exegesis] Ignore X86 pseudo instructions..
Nov 6 2018, 5:59 AM
gchatelet added inline comments to D54147: [llvm-exegesis] Ignore X86 pseudo instructions..
Nov 6 2018, 5:01 AM
gchatelet added inline comments to D54144: [llvm-exegesis] Correclty handle all X86 memory encoding formats..
Nov 6 2018, 4:58 AM

Nov 5 2018

gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Nov 5 2018, 2:33 AM · Restricted Project

Oct 30 2018

gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Oct 30 2018, 3:26 PM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Address comments
Oct 30 2018, 3:24 PM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Here are 15 random ones from llvm synced at 8f9fb8bab2e9b5b27fe40d700d2abe967b99fbb5.

lib/Target/ARM/AsmParser/ARMAsmParser.cpp:3802:31: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
tools/dsymutil/MachOUtils.cpp:330:10: warning: narrowing conversion from 'unsigned long' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/X86/X86ISelDAGToDAG.cpp:1237:14: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
tools/llvm-objdump/MachODump.cpp:7998:12: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/CodeGen/MachinePipeliner.cpp:3268:11: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/CodeGen/MIRParser/MIRParser.cpp:823:41: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Analysis/MemoryBuiltins.cpp:610:59: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Analysis/ValueTracking.cpp:2291:21: warning: narrowing conversion from 'unsigned long' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/ARM/ARMLoadStoreOptimizer.cpp:1470:43: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2300:14: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/X86/X86TargetTransformInfo.cpp:2590:27: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/ARM/ARMFrameLowering.cpp:1770:27: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/ARM/ARMConstantIslandPass.cpp:514:22: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Transforms/Vectorize/LoopVectorize.cpp:5500:13: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1199:54: warning: narrowing conversion from 'int' to 'unsigned int' [cppcoreguidelines-narrowing-conversions]
Oct 30 2018, 9:16 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Remove leftover
Oct 30 2018, 9:16 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

So I ran llvm/tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py
The bare version produces 65664 unique findings.
The version restricted to cppcoreguidelines-narrowing-conversions produces 4323 unique findings. That's about +7% of findings.
I can post some random ones is you're interested.

Oct 30 2018, 8:07 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

So I've updated the code to add more narrowing conversions.
It now tests constant expressions against receiving type, do not warn about implicit bool casting, handles ternary operator with constant expressions.

Oct 30 2018, 7:28 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Add more checks, disable bool implicit casts warning, enable ternary operator warnings.
Oct 30 2018, 7:23 AM · Restricted Project

Oct 26 2018

gchatelet accepted D53766: [llvm-exegesis] Fix SNB counter definition and handling..
Oct 26 2018, 8:49 AM

Oct 25 2018

gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Did you run this code over a real-world code-base and did you find new stuff and/or false positives or the like?

Yes I did run it over our code base. I didn't find false positive but 98% of the warnings are from large generated lookup table initializers, e.g. const static float kTable[] = {0.0, 2.0, ... };
Since every number in the list triggers the warning, it accounts for most of them.

I scrutinized a few hundreds of the rest: none were actual bugs (although it's hard to tell sometimes), most are legit like float value = 0.0; but I also found some oddities from generated headers.

To me the warnings are useful and if it were my code I'd be willing to fix them. That said, I'd totally understand that many people would find them useless or annoying.
What do you think? Shall we still commit this as is?

It would be nice to know how many new findings does this patch introduce (number of findings before the patch vs after). If it is not too much, it is fine the commit as it is.
I'd suggest to run the check on llvm code repository (using clang-tidy/tool/run-clang-tidy.py, and only enable cppcoreguidelines-narrowing-conversions).

Oct 25 2018, 8:13 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Did you run this code over a real-world code-base and did you find new stuff and/or false positives or the like?

Oct 25 2018, 7:42 AM · Restricted Project

Oct 24 2018

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Join the two parts of the ReleaseNotes update
Oct 24 2018, 7:00 AM · Restricted Project
gchatelet added a comment to D53488: [clang-tidy] Improving narrowing conversions.

Please add a short notice in the release notes for this change.

Oct 24 2018, 5:43 AM · Restricted Project
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Add documentation to ReleaseNotes
Oct 24 2018, 5:08 AM · Restricted Project
gchatelet committed rL345130: [llvm-exegesis] Implements a cache of Instruction objects..
[llvm-exegesis] Implements a cache of Instruction objects.
Oct 24 2018, 4:57 AM
gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Update documentation, makes returning code path more explicit.
Oct 24 2018, 4:09 AM · Restricted Project
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Oct 24 2018, 4:09 AM · Restricted Project
gchatelet added inline comments to D53488: [clang-tidy] Improving narrowing conversions.
Oct 24 2018, 3:01 AM · Restricted Project

Oct 23 2018

gchatelet updated the diff for D53488: [clang-tidy] Improving narrowing conversions.
  • Addressing comments
Oct 23 2018, 4:43 AM · Restricted Project

Oct 22 2018

gchatelet created D53502: [llvm-exegesis] ExecutionMode is computed upfront for Uops as well.
Oct 22 2018, 8:13 AM
gchatelet committed rL344907: [llvm-exegesis] Crash when assembling invalid Operand.
[llvm-exegesis] Crash when assembling invalid Operand
Oct 22 2018, 8:08 AM
gchatelet committed rL344906: [llvm-exegesis] Mark x86 segment register instructions as unsupported..
[llvm-exegesis] Mark x86 segment register instructions as unsupported.
Oct 22 2018, 7:58 AM
gchatelet closed D53499: [llvm-exegesis] Mark x86 segment register instructions as unsupported..
Oct 22 2018, 7:57 AM
gchatelet created D53499: [llvm-exegesis] Mark x86 segment register instructions as unsupported..
Oct 22 2018, 7:56 AM
gchatelet committed rL344905: [llvm-exegesis] Reject x86 instructions that use non uniform memory accesses.
[llvm-exegesis] Reject x86 instructions that use non uniform memory accesses
Oct 22 2018, 7:48 AM
gchatelet closed D53438: [llvm-exegesis] Reject x86 instructions that use non uniform memory accesses.
Oct 22 2018, 7:48 AM
gchatelet updated the diff for D53438: [llvm-exegesis] Reject x86 instructions that use non uniform memory accesses.
  • Adding missing comment
Oct 22 2018, 2:55 AM
gchatelet updated the diff for D53438: [llvm-exegesis] Reject x86 instructions that use non uniform memory accesses.
  • Addressing comments
Oct 22 2018, 2:51 AM
gchatelet created D53488: [clang-tidy] Improving narrowing conversions.
Oct 22 2018, 2:20 AM · Restricted Project
gchatelet accepted D53455: [llvm-exegesis] Move namespace exegesis inside llvm::.

Nice ! Thx a lot :)

Oct 22 2018, 1:46 AM

Oct 19 2018

gchatelet created D53438: [llvm-exegesis] Reject x86 instructions that use non uniform memory accesses.
Oct 19 2018, 8:17 AM
gchatelet accepted D53430: [llvm-exegesis] Mark second-form X87 instructions as unsupported..
Oct 19 2018, 5:11 AM
gchatelet accepted D53429: [llvm-exegesis] Re-enable liveliness tracker..
Oct 19 2018, 3:59 AM
gchatelet accepted D53423: [llvm-exegesis] X87 RFP setup code..
Oct 19 2018, 2:50 AM

Oct 18 2018

gchatelet committed rL344731: [llvm-exegesis] Fix off by one error.
[llvm-exegesis] Fix off by one error
Oct 18 2018, 1:23 AM

Oct 17 2018

gchatelet accepted D53371: [llvm-exegesis] Allow measuring several instructions in a single run..
Oct 17 2018, 7:32 AM
gchatelet added inline comments to D53371: [llvm-exegesis] Allow measuring several instructions in a single run..
Oct 17 2018, 7:18 AM
gchatelet added a comment to D53371: [llvm-exegesis] Allow measuring several instructions in a single run..

How do you plan to deal with opcodes that crash (assertion failure) the llvm as a whole?
I.e. not when the snippet crashes, but the generation of the snippet itself crashes.
I *believe* there are some cases.

Oct 17 2018, 6:46 AM
gchatelet committed rL344692: Fix uninitialized variable.
Fix uninitialized variable
Oct 17 2018, 5:29 AM
gchatelet committed rL344690: BuildBot fix, compiler complains about array decay to pointer.
BuildBot fix, compiler complains about array decay to pointer
Oct 17 2018, 5:11 AM
gchatelet committed rL344689: [llvm-exegeis] Computing Latency configuration upfront so we can generate many….
[llvm-exegeis] Computing Latency configuration upfront so we can generate many…
Oct 17 2018, 4:40 AM
gchatelet closed D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
Oct 17 2018, 4:39 AM
gchatelet updated the diff for D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
  • Addressing comments
Oct 17 2018, 4:13 AM
gchatelet updated the diff for D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
  • Fix variable name
Oct 17 2018, 12:46 AM
gchatelet updated the diff for D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
  • Revert files from other commit to previous state
Oct 17 2018, 12:21 AM

Oct 16 2018

gchatelet updated the diff for D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
  • Remove dead code and fix comment
Oct 16 2018, 8:29 AM
gchatelet updated the diff for D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
-Addressing comments
Oct 16 2018, 6:41 AM
gchatelet created D53320: [llvm-exegeis] Computing Latency configuration upfront so we can generate many CodeTemplates at once..
Oct 16 2018, 5:07 AM

Oct 15 2018

gchatelet committed rL344496: [llvm-exegesis] Fix missing std::move..
[llvm-exegesis] Fix missing std::move.
Oct 15 2018, 2:23 AM
gchatelet committed rL344493: [llvm-exegesis][NFC] Return many CodeTemplates instead of one..
[llvm-exegesis][NFC] Return many CodeTemplates instead of one.
Oct 15 2018, 2:11 AM
gchatelet closed D53209: [llvm-exegesis][NFC] Return many CodeTemplates instead of one..
Oct 15 2018, 2:11 AM

Oct 12 2018

gchatelet updated the diff for D53209: [llvm-exegesis][NFC] Return many CodeTemplates instead of one..
  • Inlining some code.
Oct 12 2018, 11:23 AM
gchatelet created D53209: [llvm-exegesis][NFC] Return many CodeTemplates instead of one..
Oct 12 2018, 11:18 AM
gchatelet committed rL344351: [llvm-exegesis][NFC] Simplify code at the cost of small code duplication.
[llvm-exegesis][NFC] Simplify code at the cost of small code duplication
Oct 12 2018, 8:14 AM
gchatelet closed D53198: [llvm-exegesis][NFC] Simplify code at the cost of small code duplication.
Oct 12 2018, 8:14 AM
gchatelet added a comment to D53198: [llvm-exegesis][NFC] Simplify code at the cost of small code duplication.

The diffing tool makes the change difficult to review but it's basically getting rid of the templated X86SnippetGenerator.

Oct 12 2018, 8:06 AM
gchatelet created D53198: [llvm-exegesis][NFC] Simplify code at the cost of small code duplication.
Oct 12 2018, 8:01 AM