Page MenuHomePhabricator

leonardchan (Leonard Chan)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

leonardchan added a comment to D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches.

Commit message should say only if?

Mon, Mar 30, 4:24 PM · Restricted Project, Restricted Project
leonardchan retitled D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches from [NewPM] Run the Speculative Execution Pass if the target has divergent branches to [NewPM] Run the Speculative Execution Pass only if the target has divergent branches.
Mon, Mar 30, 4:24 PM · Restricted Project, Restricted Project

Thu, Mar 26

leonardchan added a comment to D76893: Move setBugReportMsg() out from under a conditional.

LGTM. Thanks for the fix. I'll go ahead and submit this.

You should have done git commit --amend --author='Paul Robinson <...>' to credit this change properly to Paul.

Thu, Mar 26, 6:00 PM · Restricted Project
leonardchan committed rG5d929e66469d: Move setBugReportMsg() out from under a conditional (authored by leonardchan).
Move setBugReportMsg() out from under a conditional
Thu, Mar 26, 4:53 PM
leonardchan closed D76893: Move setBugReportMsg() out from under a conditional.
Thu, Mar 26, 4:53 PM · Restricted Project
leonardchan accepted D76893: Move setBugReportMsg() out from under a conditional.

LGTM. Thanks for the fix. I'll go ahead and submit this.

Thu, Mar 26, 4:52 PM · Restricted Project
leonardchan added a comment to D74324: Tools emit the bug report URL on crash.

Hi. This patch seems to cause the undefined symbol error we're seeing when linking clang:

Thu, Mar 26, 11:55 AM · Restricted Project, Restricted Project
leonardchan accepted D76856: Fix TBAA for unsigned fixed-point types.

LGTM. Thanks for the fix!

Thu, Mar 26, 11:55 AM · Restricted Project
leonardchan planned changes to D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.
Thu, Mar 26, 11:24 AM · Restricted Project, Restricted Project

Wed, Mar 25

leonardchan added a comment to D76651: [compiler-rt] Refactor FuzzedDataProvider for better readability..
Wed, Mar 25, 11:16 PM · Restricted Project
leonardchan added a comment to D76651: [compiler-rt] Refactor FuzzedDataProvider for better readability..

Hi. I think this patch might be leading to the linker error we're seeing:

Wed, Mar 25, 11:53 AM · Restricted Project

Mon, Mar 23

leonardchan added a comment to D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments.

Hi @leonardchan ,

I've double-checked the Neon intrinsics reference and it indeed confirms that the only allowed value for the lane argument for vdupq_lane_f64 is 0:

Argument Preparation

vec → Vn.1D 
0 << lane << 0

(https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vdupq_lane_f64)

I believe the observed behaviour is expected.

Mon, Mar 23, 1:06 PM · Restricted Project
leonardchan accepted D76119: [Coroutines] Insert lifetime intrinsics even O0 is used.

hi, @leonardchan, It seems that you disabled lifetime insertion at https://reviews.llvm.org/D63153. Is there any other reason to disable it beyond fix testcases? Also, could you please review this? we need lifetime check to prevent potential ‘use-after-free’ issue described at https://reviews.llvm.org/D75664

Mon, Mar 23, 1:06 PM · Restricted Project

Fri, Mar 20

leonardchan added a comment to D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments.

Actually this may not need a reproducer since the error seems straightforward with just calling a builtin function. The main issue is that before this patch, something like

Fri, Mar 20, 4:18 PM · Restricted Project
leonardchan added a comment to D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments.

Hi, a bisect seems to show https://reviews.llvm.org/rGf56550cf7f12b581a237b48a7f4d8b6682d45a09 is causing us to see the following error:

Fri, Mar 20, 3:12 PM · Restricted Project

Thu, Mar 19

leonardchan updated subscribers of D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.

Putting this on hold for now. Although this implementation works for ASan, it would be have to be repeated for other tools like SourceBasedCoverage or other sanitizers.

Thu, Mar 19, 7:15 PM · Restricted Project, Restricted Project
leonardchan retitled D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation from [ASan] Apply -ffile-prefix-map mappings to ASan instrumentation to [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.
Thu, Mar 19, 7:12 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.
Thu, Mar 19, 11:27 AM · Restricted Project, Restricted Project

Wed, Mar 18

leonardchan updated the diff for D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.
Wed, Mar 18, 11:57 PM · Restricted Project, Restricted Project
leonardchan created D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation.
Wed, Mar 18, 11:57 PM · Restricted Project, Restricted Project
leonardchan added a comment to D76018: [CMake] Enable the use of -ffile-prefix-map.

Oh, I think you'll need to add it to llvm/cmake/modules/LLVMExternalProjectUtils.cmake also:

Wed, Mar 18, 9:09 PM · Restricted Project
leonardchan created D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches.
Wed, Mar 18, 3:13 PM · Restricted Project, Restricted Project
leonardchan added a comment to D74809: [MBP][X86] Include static prof data when collecting loop BBs.

Is it expected that this patch could increase binary size? I'm seeing about a 9kB total increase in fuchsia ZBIs between clang with this patch and the one before it.

Wed, Mar 18, 12:29 PM · Restricted Project

Tue, Mar 17

leonardchan added a comment to D74825: SpeculativeExecution: fixed ingoring free execution.

Is it expected for binary size increases to result from this? Between the commit for this patch and the commit before it, I'm seeing an increase in some fuchsia ZBIs by about 13 kB.

The pass isn't supposed to even run for CPU targets

I think this pass is also running for non-GPU targets. I'm seeing this pass run on x86_64, aarch64, and riscv64 when building a toolchain for those targets.

https://github.com/llvm/llvm-project/blob/a4cde9ad7b6f1a4cfef228f6cf2fc4911bf24c77/llvm/lib/Passes/PassBuilder.cpp#L436 seems to add it to the new PM default function pipeline that I think runs as long as optimizations are available.

That doesn't match what the comment says, or the old PM does (which does createSpeculativeExecutionIfHasBranchDivergencePassz)

Tue, Mar 17, 6:56 PM · Restricted Project
leonardchan added a comment to D74825: SpeculativeExecution: fixed ingoring free execution.

Is it expected for binary size increases to result from this? Between the commit for this patch and the commit before it, I'm seeing an increase in some fuchsia ZBIs by about 13 kB.

The pass isn't supposed to even run for CPU targets

Tue, Mar 17, 5:18 PM · Restricted Project
leonardchan added a comment to D74825: SpeculativeExecution: fixed ingoring free execution.

Is it expected for binary size increases to result from this? Between the commit for this patch and the commit before it, I'm seeing an increase in some fuchsia ZBIs by about 13 kB.

Tue, Mar 17, 2:35 PM · Restricted Project

Mon, Mar 16

leonardchan accepted D76189: [Fuchsia] Use -ffile-prefix-map.
Mon, Mar 16, 12:34 PM · Restricted Project
leonardchan accepted D76204: [Fuchsia] Include llvm-gsymutil tool in the Fuchsia toolchain.
Mon, Mar 16, 12:34 PM · Restricted Project

Wed, Mar 11

leonardchan accepted D76018: [CMake] Enable the use of -ffile-prefix-map.

I also tried this but am still finding some paths embedded in libunwind and libclang_rt for some reason.

Wed, Mar 11, 12:29 PM · Restricted Project

Tue, Mar 10

leonardchan committed rG1c70dec18c7e: [libunwind] Remove __FILE__ and __LINE__ from error reporting (authored by leonardchan).
[libunwind] Remove __FILE__ and __LINE__ from error reporting
Tue, Mar 10, 7:10 PM
leonardchan closed D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting.
Tue, Mar 10, 7:10 PM · Restricted Project, Restricted Project
leonardchan added a comment to D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting.

__FILE__ only expands to an absolute path if you pass an absolute path to clang (ctrl-f "FILE" on http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html).

It feels like this is working around a deficiency in the compiler. Maybe fixing whatever's going wrong in the compiler might be better? (There's also -ffile-file-prefix map, but using that makes your _commandline_ depend on the name of the build dir, and (in clang) __FILE_NAME__ .

Tue, Mar 10, 2:46 PM · Restricted Project, Restricted Project
leonardchan updated the diff for D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting.
Tue, Mar 10, 12:33 PM · Restricted Project, Restricted Project
leonardchan retitled D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting from [libunwind] Add flag to disable logging to [libunwind] Remove __FILE__ and __LINE__ from error reporting.
Tue, Mar 10, 12:33 PM · Restricted Project, Restricted Project

Mon, Mar 9

leonardchan created D75890: [libunwind] Remove __FILE__ and __LINE__ from error reporting.
Mon, Mar 9, 9:12 PM · Restricted Project, Restricted Project

Feb 26 2020

leonardchan added a comment to D73743: Inline basic_string::erase for fastpath where __n == npos.

Hi, I believe this patch is causing the following undefined symbol error we're seeing in our 2 stage mac builders:

Feb 26 2020, 6:25 PM · Restricted Project
leonardchan committed rGd2cbaf1755ff: Fix for the test failues introduced in https://reviews.llvm.org/D74704. (authored by leonardchan).
Fix for the test failues introduced in https://reviews.llvm.org/D74704.
Feb 26 2020, 12:27 PM
leonardchan added a comment to D74704: Support -fuse-ld=lld for riscv.

Submitted fixes in d2cbaf1755ffa90300365c0d71400a5ee4ada3bd

Feb 26 2020, 12:26 PM · Restricted Project
leonardchan added a comment to D74704: Support -fuse-ld=lld for riscv.

Unsure if my reply was sent over email so copying it here.

Feb 26 2020, 12:08 PM · Restricted Project
leonardchan added a comment to D68480: Implementation of C++20's P1135R6 for libcxx.

@leonardchan @phosek Is there a way that you could add a libc++ builder that runs freestanding?

Feb 26 2020, 11:41 AM · Restricted Project
leonardchan added a comment to D74704: Support -fuse-ld=lld for riscv.

Hi again, I think e058667a2e017d3225a9bb067dbac7f2159576f7 might've broken our toolchain again:

Feb 26 2020, 10:25 AM · Restricted Project
leonardchan added a comment to D74883: Add a llvm-gsymutil tool that can convert object files to GSYM and perform lookups..

Hi, this seems to have led to a test failure on our clang bots:

Feb 26 2020, 10:15 AM · Restricted Project
leonardchan added a comment to D68480: Implementation of C++20's P1135R6 for libcxx.

Hi, a bisect shows that this patch seems to cause time.h to not be found for us:

Feb 26 2020, 10:13 AM · Restricted Project

Feb 25 2020

leonardchan added a comment to D69876: Support output constraints on "asm goto".

Woops, sorry. Didn't see you sent out https://reviews.llvm.org/rGe11f9fb4508534d31b09d2ba6cd22428ccc75f65.

Feb 25 2020, 12:10 PM · Restricted Project, Restricted Project
leonardchan added a comment to D69876: Support output constraints on "asm goto".

*Bump* We're also seeing the same test fail on our aarch64 bots: https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8887516314819686496?blamelist=1#blamelist-tab

Feb 25 2020, 12:01 PM · Restricted Project, Restricted Project

Feb 24 2020

leonardchan added a comment to D74736: [ELF] Ignore the maximum of input section alignments for two cases.

Hi, a bisect seems to show that this causes a kernel panic for an OOM test we have for zircon (https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8887545931153261696/+/steps/failures/0/steps/Linux-_2_/0/steps/attempt_0__fail_/0/steps/failed:_host_x64_oom_tests/0/logs/stdio/0).

Feb 24 2020, 6:15 PM · Restricted Project

Feb 20 2020

leonardchan accepted D71550: [Intrinsic] Add fixed point saturating division intrinsics..

Ah I see. Could you clarify though on how handling MIN / -EPS invokes undefined behavior?

It's the same as if we had MIN / -1 in integer arithmetic. The result is MAX+1, so we get overflow, which is UB. The same case happens for our lowering of scaled division, since if we have MIN / -EPS (-EPS is -1 in the integer representation), during widening/promotion we may potentially shift up the LHS by the amount widened, giving us MIN / -EPS again, which causes overflow when we do the integer division.

This doesn't matter for non-saturating division since the operation overflows there as well, but for saturating division we need to check the result and saturate, but the div instruction will cause an exception so we never get to that stage.

Also, if handling this UB requires an extra bit, it seems odd that we would add one a second time. I could be wrong, but isn't the reason for adding the 1st extra bit (i64 -> i65) is just to force the operands to be promoted? If so, then by the time we check LHSLead + RHSTrail < Scale + (unsigned)(Saturating && Signed)) in expandFixedPointDiv, we would've already gotten the extra bit from the initial promotion.

Yes, we add the extra bit to force promotion, but we have to shift up and consume that bit in order to preserve the saturating behavior (SelectionDAGBuilder.cpp:5477) so we don't win any leading sign bits there. We only get those when we do the promotion, but we are one bit short there because of the pre-promotion, so we end up having to widen again. expandFixedPointDiv (during promotion) and earlyExpandDIVFIX do not need to do this extra shifting since they handle saturation directly.

The premature promotion really makes fixed-point division overly complicated.

I feel like we should only need to promote once when doing fixed point division.

You are right in that the pre-promotion causes us to lose out on a bit. It would maybe be possible to skip the aforementioned one-bit shift, but then we have to tell promotion that we need one extra bit of saturation, and I don't know how to effectively propagate that information.

Feb 20 2020, 12:01 PM · Restricted Project
leonardchan added a comment to D74785: [AArch64][SVE] Add intrinsics for SVE2 bitwise ternary operations.

Hi, I think this might've caused some test failures on our bots and others on buildbot:

Feb 20 2020, 10:47 AM · Restricted Project

Feb 19 2020

leonardchan added a comment to D74791: Add a --shuffle-sections=seed option to lld.

Hi, I think this might be triggering the test failures seen on buildbot with:

Feb 19 2020, 4:04 PM · Restricted Project
leonardchan added inline comments to D73186: [AST] Add fixed-point multiplication constant evaluation..
Feb 19 2020, 11:55 AM · Restricted Project

Feb 18 2020

leonardchan added a comment to D74704: Support -fuse-ld=lld for riscv.

Hello @serge-sans-paille,

looks like these changes broke the ARM builders:
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/4749
http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/4752

failed tests:

  • FAIL: Clang::riscv64-toolchain.c
  • FAIL: Clang::riscv32-toolchain.c

    with the following error: ` C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Driver\riscv64-toolchain.c:15:27: error: C-RV64-BAREMETAL-LP64: expected string not found in input `

    would you fix the problem or revert the commit?
Feb 18 2020, 11:12 AM · Restricted Project

Feb 12 2020

leonardchan added a comment to D73329: [libFuzzer] communicate through pipe to subprocess for MinimizeCrashInput.

Hi, a bisect seems to show that this patch resulted in a series of linker errors when building fuzzers fuchsia:

 [48364/49380] LINK user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer
 FAILED: user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.debug user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.map
 ../../../recipe_cleanup/clang9HU4mg/bin/clang++ -o user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.debug -fuse-ld=lld -Wl,--threads -Wl,-z,combreloc -Wl,-z,relro -Wl,-z,now -Wl,-z,text -Wl,--pack-dyn-relocs=relr --target=x86_64-fuchsia -mcx16 -march=x86-64 -fcrash-diagnostics-dir=clang-crashreports -fcolor-diagnostics -Wl,--color-diagnostics -Wl,-z,max-page-size=4096 -ffile-prefix-map=/b/s/w/ir/k/fuchsia/out/default.zircon=. -ffile-prefix-map=/b/s/w/ir/k/fuchsia/out=.. -ffile-prefix-map=/b/s/w/ir/k/fuchsia=../.. -no-canonical-prefixes -O2 -Wl,--icf=all -g3 -ffunction-sections -Wl,--gc-sections -fdata-sections -fno-exceptions -fno-rtti -nostartfiles -nolibc -Wl,-dynamic-linker=fuzzer.asan-ubsan/ld.so.1 -fsanitize=fuzzer -fsanitize=undefined -fsanitize=address -L../../zircon/public/gn/config/libc-dummy -Wl,-Map,user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.map -Wl,--start-group @'user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.rsp' -lunwind -Wl,--end-group user.vdso-x64-clang.shlib/obj/system/ulib/zircon/libzircon.so.debug user.fuzzer-x64-asan-ubsan.shlib/obj/system/ulib/c/libc.so.debug user-x64-asan-ubsan.shlib/obj/system/ulib/fdio/libfdio.so.debug && ../../../recipe_cleanup/clang9HU4mg/bin/llvm-objcopy --strip-sections "user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.debug" "user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer" && case '-fuse-ld=lld -Wl,--threads -Wl,-z,combreloc -Wl,-z,relro -Wl,-z,now -Wl,-z,text -Wl,--pack-dyn-relocs=relr --target=x86_64-fuchsia -mcx16 -march=x86-64 -fcrash-diagnostics-dir=clang-crashreports -fcolor-diagnostics -Wl,--color-diagnostics -Wl,-z,max-page-size=4096 -ffile-prefix-map=/b/s/w/ir/k/fuchsia/out/default.zircon=. -ffile-prefix-map=/b/s/w/ir/k/fuchsia/out=.. -ffile-prefix-map=/b/s/w/ir/k/fuchsia=../.. -no-canonical-prefixes -O2 -Wl,--icf=all -g3 -ffunction-sections -Wl,--gc-sections -fdata-sections -fno-exceptions -fno-rtti -nostartfiles -nolibc -Wl,-dynamic-linker=fuzzer.asan-ubsan/ld.so.1 -fsanitize=fuzzer -fsanitize=undefined -fsanitize=address -L../../zircon/public/gn/config/libc-dummy' in *build-id=none*) ;; *) ../../prebuilt/tools/buildidtool/linux-x64/buildidtool -build-id-dir ".build-id" -stamp "user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.build-id.stamp" -entry "=user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer" -entry ".debug=user.fuzzer-x64-asan-ubsan/obj/system/ulib/elfload/test/elfload-fuzzer.debug" ;; esac
 ld.lld: error: undefined symbol: fuzzer::OpenProcessPipe(char const*, char const*)
>>> referenced by FuzzerDriver.cpp:321 (compiler-rt/lib/fuzzer/FuzzerDriver.cpp:321)
>>>               fuzzer.o:(fuzzer::ExecuteCommandWithPopen(fuzzer::Command const&, std::__Fuzzer::basic_string<char, std::__Fuzzer::char_traits<char>, std::__Fuzzer::allocator<char> >*)) in archive ../../../recipe_cleanup/clang9HU4mg/lib/clang/11.0.0/lib/x86_64-unknown-fuchsia/libclang_rt.fuzzer.a
 
 ... and many more

Could you take a look or revert this patch? Thanks.

I tested and if there will be a revert, 4f3c3bbbf85a1283796e0e80c654779e40ce328e may also need to be reverted since it depends on this patch.

Builder link: https://ci.chromium.org/p/fuchsia/builders/ci/clang_toolchain.fuchsia-x64-debug-subbuild/b8888652956426122432

Apologies that I did not add OpenProcessPipe implementation on Fushia. Quick question is there popen/pclose on Fushia? Thanks.

Feb 12 2020, 2:24 PM · Restricted Project, Restricted Project
leonardchan added a comment to D73329: [libFuzzer] communicate through pipe to subprocess for MinimizeCrashInput.

Hi, a bisect seems to show that this patch resulted in a series of linker errors when building fuzzers fuchsia:

Feb 12 2020, 2:05 PM · Restricted Project, Restricted Project
leonardchan added a comment to D74240: [NFC] Refactor the tuple of symbol information with structure for llvm-objdump.

Actually, disregard my last comment. I think I might've made a mistake on the bisect. Sorry!

Feb 12 2020, 1:37 PM · Restricted Project
leonardchan added a comment to D74240: [NFC] Refactor the tuple of symbol information with structure for llvm-objdump.
Feb 12 2020, 1:02 PM · Restricted Project

Feb 10 2020

leonardchan added a comment to rGda3dc0011e06: PR44684: Look through parens and similar constructs when determining whether a….

Hi, a bisect seems to show that this patche causes the build error we're seeing at https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8889138534807714304/+/steps/build/0/steps/build_fuchsia/0/steps/ninja/0/steps/zircon/0/logs/raw_io.output_failure_raw_summary_/0

Feb 10 2020, 8:01 PM

Feb 4 2020

leonardchan added a comment to D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.

This broke some of our downstream tests using vla:s. Those are compiled from C and executed in gdb. What happens is that DW_OP_deref disappears and then the debugger prints the wrong value.

Feb 4 2020, 11:53 AM · debug-info, Restricted Project
leonardchan added a comment to D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.

I'll be able to take a look at this on Friday, but in the meantime if there's no fixes for this and waiting until then is too long then it's probably better to revert this. Sorry for the breakage.

Feb 4 2020, 11:44 AM · debug-info, Restricted Project

Jan 31 2020

leonardchan accepted D73810: [CMake][Fuchsia] Support for building on Windows.
Jan 31 2020, 3:21 PM · Restricted Project

Jan 30 2020

leonardchan committed rG2d3174c4df6b: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location (authored by leonardchan).
[SafeStack][DebugInfo] Insert DW_OP_deref in correct location
Jan 30 2020, 5:19 PM
leonardchan committed rG3b23453b6cbf: Revert "[SafeStack][DebugInfo] Insert DW_OP_deref in correct location" (authored by leonardchan).
Revert "[SafeStack][DebugInfo] Insert DW_OP_deref in correct location"
Jan 30 2020, 4:23 PM
leonardchan added a reverting change for rGfff6a1b0f1fe: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location: rG3b23453b6cbf: Revert "[SafeStack][DebugInfo] Insert DW_OP_deref in correct location".
Jan 30 2020, 4:23 PM
leonardchan committed rGfff6a1b0f1fe: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location (authored by leonardchan).
[SafeStack][DebugInfo] Insert DW_OP_deref in correct location
Jan 30 2020, 4:05 PM
leonardchan closed D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.
Jan 30 2020, 4:05 PM · debug-info, Restricted Project
leonardchan added a comment to D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.

*ping* Can we land this or revert D68945 in the meantime?

Jan 30 2020, 11:35 AM · debug-info, Restricted Project

Jan 29 2020

leonardchan added inline comments to D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.
Jan 29 2020, 9:49 AM · debug-info, Restricted Project

Jan 28 2020

leonardchan added inline comments to D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.
Jan 28 2020, 11:10 AM · debug-info, Restricted Project

Jan 27 2020

leonardchan created D73526: [SafeStack][DebugInfo] Insert DW_OP_deref in correct location.
Jan 27 2020, 8:24 PM · debug-info, Restricted Project

Jan 23 2020

leonardchan added inline comments to D73186: [AST] Add fixed-point multiplication constant evaluation..
Jan 23 2020, 5:47 PM · Restricted Project
leonardchan accepted D73182: [CodeGen] Emit IR for fixed-point multiplication and division..
Jan 23 2020, 4:21 PM · Restricted Project
leonardchan accepted D73184: [CodeGen] Emit IR for compound assignment with fixed-point operands..
Jan 23 2020, 4:11 PM · Restricted Project
leonardchan accepted D73183: [CodeGen] Emit IR for fixed-point unary operators..
Jan 23 2020, 4:02 PM · Restricted Project
leonardchan added a comment to D71550: [Intrinsic] Add fixed point saturating division intrinsics..

Ah, that's unfortunate... It's a consequence of being forced to handle the integer division overflow case gracefully.

With an i64, we get a sext+shl and an i65 after building. During the initial promotion we try to promote to i128. That gives us 63 bits of redundant sign (128-65), but in order to handle the case of MSB / ALL1 (MIN / -EPS) without invoking undefined behavior in the integer division, we require an extra bit. Since we don't have that bit to spare, we end up not being able to do the operation in i128. Then we get an i256 sdiv and there's no codegen for that.

I'm unsure what to do to make this look good. It would be convenient to skip the operation altogether in the overflow case, but we can't generate control flow here. I could check for the overflow case and fudge the inputs/result to prevent the overflow, but that feels sort of wrong. Might be the only viable option, though.

Jan 23 2020, 3:52 PM · Restricted Project

Jan 22 2020

leonardchan accepted D73188: [AST] Improve overflow diagnostics for fixed-point constant evaluation..

Could you also add a few test lines showing this warning doesn't appear for operations where the result is a saturated type?

Jan 22 2020, 5:32 PM · Restricted Project
leonardchan accepted D73189: [AST] Fix certain consteval assignment and comma operator issues with fixed-point types..
Jan 22 2020, 5:32 PM · Restricted Project
leonardchan added a comment to D71550: [Intrinsic] Add fixed point saturating division intrinsics..

It seems that I run into LLVM ERROR: Unsupported library call operation! if I call llvm.sdiv.fix.sat.i64 with a scale of 63:

Jan 22 2020, 3:16 PM · Restricted Project

Jan 17 2020

leonardchan created D72959: Relative VTables ABI on Fuchsia.
Jan 17 2020, 2:15 PM · Restricted Project, Restricted Project
leonardchan added a comment to D68945: [DebugInfo] Don't translate dbg.addr and similar intrinsics into indirect DBG_VALUEs.

Hi, it seems that this revision introduced the regression in https://bugs.llvm.org/show_bug.cgi?id=44585 that produces incorrect DWARF expressions when using safestack. Would you mind taking a look? Thanks.

Jan 17 2020, 10:03 AM · Restricted Project

Jan 13 2020

leonardchan accepted D72537: [NewPM] Port MergeFunctions pass.

LGTM with just one quick question

Jan 13 2020, 11:03 AM · Restricted Project

Jan 10 2020

leonardchan added a comment to rG61bd19206f61: [libc++] Explicitly enumerate std::string external instantiations..

Hi, it seems that this commit also introduces libc++ headers that are incompatible with gcc 9.2:

Jan 10 2020, 2:06 PM

Jan 9 2020

leonardchan added a comment to D71550: [Intrinsic] Add fixed point saturating division intrinsics..

LGTM for the code so far. Just want to confirm on my side that the codegen works as intended also.

Jan 9 2020, 5:41 PM · Restricted Project
leonardchan added a comment to D71795: [ELF] Delete an unused special rule from isStaticLinkTimeConstant. NFC.

Yup, was able to confirm this is the faulty patch. Would you mind looking into this or reverting in the meantime?

Jan 9 2020, 5:04 PM · Restricted Project
leonardchan added a comment to D71795: [ELF] Delete an unused special rule from isStaticLinkTimeConstant. NFC.

*Moved from the commit to here*

Jan 9 2020, 4:26 PM · Restricted Project
leonardchan added a comment to rGb841e119d77e: [ELF] Delete an unused special rule from isStaticLinkTimeConstant. NFC.
Jan 9 2020, 4:25 PM

Dec 20 2019

leonardchan added a comment to D70919: [Hexagon] Avoid passing unsupported options to lld when -fuse-ld=lld is used.

Hi! This seems to be causing some test failures on our bots: https://ci.chromium.org/p/fuchsia/builders/ci/clang-linux-x64/b8893521019883752048

Dec 20 2019, 4:39 PM · Restricted Project, Restricted Project

Dec 6 2019

leonardchan accepted D70691: Optimization record for bytecode input missing- PR44000.

LGTM. Could you commit this soon to get the bots green again?

Dec 6 2019, 3:30 PM · Restricted Project
leonardchan added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Hi! I believe the commit for this (a3b2552575d3c333e928446fac10cc5b0b4092a9) is causing a test failure on our bots (https://ci.chromium.org/p/fuchsia/builders/ci/clang-linux-x64/b8894791990609720880) and some buildbots (can't find the link since it scrolled past the end of the console). Could you look into it when you get a chance?

Dec 6 2019, 12:24 PM · Restricted Project
leonardchan added a comment to D70007: [Intrinsic] Add fixed point division intrinsics..

Alrighty, it looks like everything in this patch should be ok after those 2 changes. I can send you the script I used to generate tests if you'd like to use it also.

Dec 6 2019, 12:15 PM · Restricted Project
leonardchan accepted D71135: [cxx][cxxabi] Don't link libpthread and libdl on Fuchsia.
Dec 6 2019, 11:18 AM · Restricted Project

Dec 4 2019

leonardchan added inline comments to D70007: [Intrinsic] Add fixed point division intrinsics..
Dec 4 2019, 4:50 PM · Restricted Project
leonardchan added inline comments to D70007: [Intrinsic] Add fixed point division intrinsics..
Dec 4 2019, 2:10 PM · Restricted Project

Nov 27 2019

leonardchan added a comment to D70007: [Intrinsic] Add fixed point division intrinsics..

Sorry for the holdup. Right now I'm just writing a bunch of tests to validate the IR expansion produces the correct division results. As of now, the patch LGTM, but I'd prefer holding off submitting the revision until I finish validating (unless you think it's better to submit now then fix any potential bugs that may be found later since I don't think anyone will be immediately using this). Here's a few comments I have so far:

Nov 27 2019, 2:36 PM · Restricted Project

Nov 26 2019

leonardchan accepted D70747: [Fuchsia] Don't fail for unknown architectures.
Nov 26 2019, 6:52 PM · Restricted Project

Nov 25 2019

leonardchan accepted D70575: [Clang] Define Fuchsia C++ABI.
Nov 25 2019, 10:16 AM · Restricted Project

Nov 21 2019

leonardchan added a comment to D70575: [Clang] Define Fuchsia C++ABI.

Could you add a test to show that with a fuchsia target we end up returning this from constructors + destructors and ensure that this ABI is used?

Nov 21 2019, 5:41 PM · Restricted Project
leonardchan accepted D70576: [Clang] Always set -z now linker option on Fuchsia.

LGTM. But just to confirm, did you test this on CQ/bots by adding ["now", "-z"] as linker flags to the config("compiler") in Fuchsia + Zircon?

Nov 21 2019, 5:14 PM · Restricted Project

Nov 20 2019

leonardchan added a comment to D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline..

Hi! I think this patch might be causing some test failures on our mac bots:

Nov 20 2019, 4:07 PM · Restricted Project
leonardchan accepted D70477: [Clang] Enable RISC-V support for Fuchsia.
Nov 20 2019, 10:21 AM · Restricted Project

Nov 18 2019

leonardchan committed rG66b6b9276521: Revert "implement printing out raw section data of xcoff objectfile for llvm… (authored by leonardchan).
Revert "implement printing out raw section data of xcoff objectfile for llvm…
Nov 18 2019, 8:08 PM
leonardchan added a reverting change for rG8f8a9f3437d4: implement printing out raw section data of xcoff objectfile for llvm-objdump: rG66b6b9276521: Revert "implement printing out raw section data of xcoff objectfile for llvm….
Nov 18 2019, 8:08 PM