leonardchan (Leonard Chan)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 25 2018, 1:47 PM (25 w, 1 d)

Recent Activity

Yesterday

leonardchan accepted D53381: [clang-doc] Bringing bitcode tests in line.
Wed, Oct 17, 11:53 AM · Restricted Project
leonardchan committed rL344702: Fix for arm bots afternew PM pass port. Prevent cross compiling on arm..
Fix for arm bots afternew PM pass port. Prevent cross compiling on arm.
Wed, Oct 17, 11:14 AM
leonardchan committed rC344702: Fix for arm bots afternew PM pass port. Prevent cross compiling on arm..
Fix for arm bots afternew PM pass port. Prevent cross compiling on arm.
Wed, Oct 17, 11:14 AM
leonardchan committed rC344701: Fix for failing unit tests on some bots after r344696..
Fix for failing unit tests on some bots after r344696.
Wed, Oct 17, 9:23 AM
leonardchan committed rL344701: Fix for failing unit tests on some bots after r344696..
Fix for failing unit tests on some bots after r344696.
Wed, Oct 17, 9:23 AM
leonardchan added a comment to D53340: [Intrinsic] Unigned Saturation Addition Intrinsic.

Is this intrinsic really needed for the fixed point support?

Maybe I've missed something in the discussions about support for unsigned fixed point types, but in our implementation the unsigned types are using the positive range of the signed type, such as [0, SIGNED_MAX]. So we keep the sign bit zero for the unsigned fixed point types. Are you planning for something different here?

In our implementation we lower addition of two saturated unsigned fixed point types to sadd.sat (if the input values are in the range [0, SIGNED_MAX] the result will be in the range [0, SIGNED_MAX] as well). So we haven't really found a need for implementing uadd.sat.

Wed, Oct 17, 9:01 AM
leonardchan updated the diff for D53340: [Intrinsic] Unigned Saturation Addition Intrinsic.
Wed, Oct 17, 9:01 AM
leonardchan committed rC344699: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with….
[PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with…
Wed, Oct 17, 8:41 AM
leonardchan committed rL344699: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with….
[PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with…
Wed, Oct 17, 8:40 AM
leonardchan closed D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Wed, Oct 17, 8:40 AM · Restricted Project, Restricted Project

Tue, Oct 16

leonardchan committed rL344659: [Sanitizer][PassManager] Fix for failing ASan tests on arm-linux-gnueabihf.
[Sanitizer][PassManager] Fix for failing ASan tests on arm-linux-gnueabihf
Tue, Oct 16, 5:18 PM
leonardchan closed D53350: [Sanitizer][PassManager] Fix for failing ASan tests on ARM.
Tue, Oct 16, 5:18 PM · Restricted Project
leonardchan created D53350: [Sanitizer][PassManager] Fix for failing ASan tests on ARM.
Tue, Oct 16, 4:44 PM · Restricted Project
leonardchan added a dependency for D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast: D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types.
Tue, Oct 16, 2:21 PM · Restricted Project
leonardchan added a dependent revision for D46917: [Fixed Point Arithmetic] Comparison and Unary Operations for Fixed Point Types: D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast.
Tue, Oct 16, 2:21 PM · Restricted Project
leonardchan created D53340: [Intrinsic] Unigned Saturation Addition Intrinsic.
Tue, Oct 16, 2:09 PM
leonardchan accepted D53085: [clang-doc] Add unit tests for Markdown.
Tue, Oct 16, 11:22 AM · Restricted Project
leonardchan accepted D53084: [clang-doc] Add unit tests for YAML.
Tue, Oct 16, 11:21 AM · Restricted Project
leonardchan updated the diff for D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast.
Tue, Oct 16, 11:09 AM · Restricted Project
leonardchan added inline comments to D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast.
Tue, Oct 16, 11:09 AM · Restricted Project
leonardchan added a comment to D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .

@philip.pfaffe @fedor.sergeev Do you have any more comments on this patch?

Tue, Oct 16, 10:38 AM · Restricted Project, Restricted Project
leonardchan committed rL344629: [Intrinsic] Signed Saturation Addition Intrinsic.
[Intrinsic] Signed Saturation Addition Intrinsic
Tue, Oct 16, 10:37 AM
leonardchan closed D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Tue, Oct 16, 10:37 AM

Mon, Oct 15

leonardchan created D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast.
Mon, Oct 15, 6:09 PM · Restricted Project
leonardchan committed rL344549: [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
[Fixed Point Arithmetic] Fix for clang-tools-extra warning
Mon, Oct 15, 1:02 PM
leonardchan committed rCTE344549: [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
[Fixed Point Arithmetic] Fix for clang-tools-extra warning
Mon, Oct 15, 1:02 PM
leonardchan closed D53299: [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
Mon, Oct 15, 1:02 PM · Restricted Project
leonardchan committed rCTE344548: added fix.
added fix
Mon, Oct 15, 1:02 PM
leonardchan committed rL344548: added fix.
added fix
Mon, Oct 15, 1:01 PM
leonardchan updated the diff for D53299: [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
Mon, Oct 15, 12:49 PM · Restricted Project
leonardchan retitled D53299: [Fixed Point Arithmetic] Fix for clang-tools-extra warning from [Fixed Point Arithmetic] to [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
Mon, Oct 15, 12:32 PM · Restricted Project
leonardchan added a comment to rL344530: [Fixed Point Arithmetic] FixedPointCast.

Sorry for the delay. Made patch https://reviews.llvm.org/D53299. Not
sure if I needed to make a patch for this or can commit right away,
but wanted to run it by you first.

Mon, Oct 15, 12:27 PM
leonardchan created D53299: [Fixed Point Arithmetic] Fix for clang-tools-extra warning.
Mon, Oct 15, 12:27 PM · Restricted Project
leonardchan added a comment to rL344530: [Fixed Point Arithmetic] FixedPointCast.

I found those earlier when working on the monorepo. In fixing them now.

Mon, Oct 15, 11:45 AM
leonardchan added inline comments to D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Mon, Oct 15, 10:05 AM
leonardchan updated the diff for D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Mon, Oct 15, 10:05 AM
leonardchan committed rC344530: [Fixed Point Arithmetic] FixedPointCast.
[Fixed Point Arithmetic] FixedPointCast
Mon, Oct 15, 9:09 AM
leonardchan committed rL344530: [Fixed Point Arithmetic] FixedPointCast.
[Fixed Point Arithmetic] FixedPointCast
Mon, Oct 15, 9:09 AM
leonardchan closed D50616: [Fixed Point Arithmetic] FixedPointCast.
Mon, Oct 15, 9:09 AM · Restricted Project
leonardchan updated the diff for D50616: [Fixed Point Arithmetic] FixedPointCast.
Mon, Oct 15, 9:06 AM · Restricted Project

Fri, Oct 12

leonardchan updated the diff for D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Fri, Oct 12, 4:28 PM · Restricted Project, Restricted Project
leonardchan added a comment to D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .

You're right, my bad; I missed the fact that it's EP_OptimizerLast, which has no equivalent in the new PM. Your implementation is actually correct here.

Did you run your test in a debug build?

Fri, Oct 12, 4:28 PM · Restricted Project, Restricted Project
leonardchan added a comment to D53053: [Intrinsic] Signed Saturation Addition Intrinsic.

Intrinsics with vector operands are expanded in LegalizeVectorOps.cpp. Sometimes with a vectorized sequence or in the worst case by unrolling.

Fri, Oct 12, 3:43 PM
leonardchan updated the diff for D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Fri, Oct 12, 3:41 PM
leonardchan updated the diff for D50616: [Fixed Point Arithmetic] FixedPointCast.
  • Removed target hook
Fri, Oct 12, 2:49 PM · Restricted Project
leonardchan updated the diff for D50616: [Fixed Point Arithmetic] FixedPointCast.
Fri, Oct 12, 2:48 PM · Restricted Project
leonardchan updated the diff for D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Fri, Oct 12, 2:06 PM
leonardchan added a comment to D53053: [Intrinsic] Signed Saturation Addition Intrinsic.

Are we not supporting vectors at all? That needs to be checked in the IR verifier if so. Though I would really like to see this for vectors since X86 has 8 and 16 bit saturating vector addition.

Fri, Oct 12, 2:06 PM

Thu, Oct 11

leonardchan added inline comments to D53170: [clang-doc] Switch to default to all-TUs executor.
Thu, Oct 11, 2:26 PM · Restricted Project
leonardchan added inline comments to D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Thu, Oct 11, 2:22 PM
leonardchan updated the diff for D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Thu, Oct 11, 2:22 PM
leonardchan committed rL344274: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new….
[PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new…
Thu, Oct 11, 11:33 AM
leonardchan closed D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Thu, Oct 11, 11:33 AM · Restricted Project

Wed, Oct 10

leonardchan added inline comments to D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Wed, Oct 10, 7:38 PM
leonardchan updated the diff for D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Wed, Oct 10, 7:38 PM
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Wed, Oct 10, 10:52 AM · Restricted Project

Tue, Oct 9

leonardchan added a comment to D50616: [Fixed Point Arithmetic] FixedPointCast.

@ebevhan @rjmccall Seeing that the saturation intrinsic in https://reviews.llvm.org/D52286 should be put on hold for now, would it be fine to submit this patch as is? Then if the intrinsic is finished, come back to this and update this patch to use the intrinsic?

Tue, Oct 9, 5:08 PM · Restricted Project
leonardchan created D53053: [Intrinsic] Signed Saturation Addition Intrinsic.
Tue, Oct 9, 4:30 PM
leonardchan added a comment to D52286: [Intrinsic] Signed Saturation Intrinsic.

If the use of a special intrinsic for saturation is rare (few targets that supports it natively), then perhaps we should put this particular patch on hold for now?

I guess the target that @bevinh (and I) is working on can override the codegen in clang to select our target specific intrinsic just like we do today. Or is it hard to do target specific overrides in clang codegen?

The generic support for fixed point types could, at least initially, use the min/max clamping. It would let us move forward on the more interesting parts of fixed point support (such as saturated add, multiplication etc), instead of getting stuck in heavy discussions regarding something that IMHO has lower priority. Then we can revisit this patch later. Perhaps it will be easier to demonstrate the need for an ssaturate intrinsic when we come further and clang is generating fixed point conversions.

Tue, Oct 9, 11:34 AM
leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

These are different per instance though. Even if merged, you're free to create one instance with options A and one instance with options B and run the first instance on a function pipeline and the second on a module pipeline.

Tue, Oct 9, 11:26 AM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 9, 11:24 AM · Restricted Project

Mon, Oct 8

leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

I meant just merging the passes. In the sense that you have one pass class with one module run() method and one function run() method. The question then is whether you'll ever want to use different options for the different IRUnits.

Mon, Oct 8, 8:10 PM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Mon, Oct 8, 8:09 PM · Restricted Project
leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

Some comments inline. Does it make sense to merge the module and function passes into one? Is it really necessary to distinguish the passes?

Mon, Oct 8, 2:22 PM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Mon, Oct 8, 2:20 PM · Restricted Project
leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

See my latest comment. If there is no state to be kept, that state need not be on the exported interface.

Mon, Oct 8, 11:50 AM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Mon, Oct 8, 11:48 AM · Restricted Project

Sun, Oct 7

leonardchan added a comment to D52286: [Intrinsic] Signed Saturation Intrinsic.

@ebevhan @craig.topper Any more comments on this patch?

Sun, Oct 7, 11:58 PM
leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

Are there any other changes that would be necessary before getting revision acceptance?

Sun, Oct 7, 11:48 PM · Restricted Project

Thu, Oct 4

leonardchan added a comment to D52386: [Lexer] Add udefined_behavior_sanitizer feature.

This sounds like something that would be better handled in the build system rather than in source code. In particular, I think you don't actually want to detect "is this translation unit being compiled with ubsan enabled?", you instead want to detect "is the rest of musl libc being compiled with ubsan enabled?" -- you should not compile the stubs themselves with ubsan enabled, because ubsan might insert calls to its runtime at arbitrary places within that translation unit, which means you'll get infinite recursion when one of the ubsan callbacks ends up calling itself.

Thu, Oct 4, 6:07 PM · Restricted Project
leonardchan added inline comments to D52286: [Intrinsic] Signed Saturation Intrinsic.
Thu, Oct 4, 3:58 PM
leonardchan updated the diff for D52286: [Intrinsic] Signed Saturation Intrinsic.
Thu, Oct 4, 3:58 PM
leonardchan added a comment to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.

Well, it will be a bit tricky to avoid moving the interfaces around, since AddressSanitizer contains quite a load of state

What kind of state is asan tracking that persists across functions or modules?

Thu, Oct 4, 2:15 PM · Restricted Project
leonardchan added a comment to D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .

Is this the right place in the pipeline to put the passes? The legacy PM inserts the asan passes at EP_OptimizerLast, why not do the same thing?

Thu, Oct 4, 12:24 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Thu, Oct 4, 12:24 PM · Restricted Project, Restricted Project

Wed, Oct 3

leonardchan added a comment to D52386: [Lexer] Add udefined_behavior_sanitizer feature.

I'm not at all convinced that this is a good thing. There isn't such a thing as "undefined behavior sanitizer". Rather, there are a whole bunch of different checks that fall under the same umbrella. This test seems on the surface to be about as meaningless as __has_feature(warnings) would be: it's useless to ask the question without knowing *which* warnings you're talking about. But perhaps there's some use case I've overlooked (and your description of the patch doesn't mention why you want this). What is the use case you're trying to address with this change?

Wed, Oct 3, 4:45 PM · Restricted Project
leonardchan added inline comments to D52847: [clang-doc] Handle anonymous namespaces.
Wed, Oct 3, 3:34 PM · Restricted Project
leonardchan accepted D52754: [clang-doc] Clean up Markdown output.
Wed, Oct 3, 3:14 PM · Restricted Project

Tue, Oct 2

leonardchan added a dependent revision for D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address : D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 2, 4:54 PM · Restricted Project, Restricted Project
leonardchan added a dependency for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager: D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Tue, Oct 2, 4:54 PM · Restricted Project
leonardchan removed a dependency for D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address : D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 2, 4:54 PM · Restricted Project, Restricted Project
leonardchan removed a dependent revision for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager: D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Tue, Oct 2, 4:54 PM · Restricted Project
leonardchan added a dependency for D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address : D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 2, 4:54 PM · Restricted Project, Restricted Project
leonardchan created D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Tue, Oct 2, 4:54 PM · Restricted Project, Restricted Project
leonardchan added a dependent revision for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager: D52814: [PassManager/Sanitizer] Enable usage of ported AddressSanitizer passes with -fsanitize=address .
Tue, Oct 2, 4:54 PM · Restricted Project
leonardchan added a comment to D52286: [Intrinsic] Signed Saturation Intrinsic.

Adding @spatel. This feels like it could be implemented with a min+max sequence in IR using 2 icmps and 2 selects. We've already spent quite a lot of effort optimizing these common patterns in both IR and SelectionDAG with the SMAX/SMIN ISD opcodes.

Tue, Oct 2, 2:26 PM
leonardchan updated the diff for D52286: [Intrinsic] Signed Saturation Intrinsic.
  • Changed second argument of SSAT node from ConstantSDNode to VTSDNode
Tue, Oct 2, 10:53 AM
leonardchan added inline comments to D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 2, 10:04 AM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Tue, Oct 2, 10:04 AM · Restricted Project

Mon, Oct 1

leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
  • Reduce size of patch by adding per-PM wrappers for AddressSanitizer and AddressSanitizerModule. The passes exposed to the new PM use opaque pointers to these wrappers.
Mon, Oct 1, 5:43 PM · Restricted Project
leonardchan updated the diff for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
  • Added LLVM header description
Mon, Oct 1, 12:21 PM · Restricted Project
leonardchan updated subscribers of D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Mon, Oct 1, 11:46 AM · Restricted Project
leonardchan removed a reviewer for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager: silvas.
Mon, Oct 1, 11:46 AM · Restricted Project
leonardchan added a reviewer for D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager: silvas.
Mon, Oct 1, 11:42 AM · Restricted Project
leonardchan created D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager.
Mon, Oct 1, 11:41 AM · Restricted Project

Fri, Sep 28

leonardchan added reviewers for D52286: [Intrinsic] Signed Saturation Intrinsic: echristo, sunfish, craig.topper.
Fri, Sep 28, 2:27 PM
leonardchan added inline comments to D52286: [Intrinsic] Signed Saturation Intrinsic.
Fri, Sep 28, 10:22 AM

Thu, Sep 27

leonardchan added inline comments to D52286: [Intrinsic] Signed Saturation Intrinsic.
Thu, Sep 27, 3:08 PM
leonardchan updated the diff for D52286: [Intrinsic] Signed Saturation Intrinsic.
Thu, Sep 27, 3:07 PM

Wed, Sep 26

leonardchan added inline comments to D52286: [Intrinsic] Signed Saturation Intrinsic.
Wed, Sep 26, 10:59 AM