Performance optimizations

Hey, that's my first Pull Request here, so please do hate me :)
I dug into the code a little, analyzing performance and trying to find potential bottlenecks.
Here's what I changed:

PageViewport

I refactored function by reducing the number of calculations within the function. I simplified some calculations on basic arithmetic level: ((b + a) / 2) - a => (b- a) / 2, a * n + b * n - c * n => (a + b - c) * n etc.
After code analysis, I've optimize the order of calculations (for example, offsetCanvasX was different for different rotation solely because width was switching with height when the viewport was rotated by 90 od 270 degrees) I've managed to reduce the code necessary to output what this function is supposed to.

That resulted in 20-40% performance increase in my tests. (~80 => 50 milliseconds per 10000 runs).

log2

I replaced custom solution with Math functions.
I placed a warning that log2 function is not exactly doing what the name suggests. Didn't change the name because I didn't want to make a commotion.

While I didn't measure how the change affected the performance in pdf.js, I did a simple test on jsperf which pointed native Math.log as a clear winner with 500% performance increase.
I've decided to use Math.ceil(Math.log(x) * Math.LOG2E) instead of Math.ceil(Math.log2(x)); for compatibility reasons; virtually every browser supports Math.log, while Math.log2 is only supported in modern browsers, while giving no performance bonus whatsoever.

CanvasGraphics_paintInlineImageXObject

I rewrote the function painting image so that it would not draw a temporary image on a canvas each time it goes through a loop which purpose is to divide widthScale and heightScale by 2 as long as they are bigger than 2.

I've measured the performance after implementing this change and the difference is big. In my test file, the function was called twice. Before the change, on average runs took 347 ms and 127 ms. After the change, they took 229 and 95 ms respectively. That's 32% performance increase. Woo! 🎉

/botio-linux preview

The default PDF file (Tracemonkey) now gives the following in the console:
Error: invalid canvas size
The second page doesn't render completely anymore because of it. I assume that this is related to the CanvasGraphics_paintInlineImageXObject change. Could you check that?

Hmmmm, yeah, I suspect the same. But I'm clicking furiously through it and I can't catch this error (but I caught several others. Do you know any more details? Does it happen on a specific page or with a specific scale?

It's triggered 100% for me when opening http://<IP_ADDRESS>:8877/6116cb2945c1db9/web/viewer.html and scrolling to page 2. It happens with any scale.

Yeah, I'm on it. It happens when scale is already at 1 so it won't ever enter this loop. I'm gonna push fixes very soon.

If I were to guess, I'd say that you probably didn't run any tests locally with these patches applied!?
Just doing a quick round of testing locally, it seems that this PR causes a number of rendering regressions in existing test-cases. Please refer to https://github.com/mozilla/pdf.js/wiki/Contributing, especially the parts about "Generating reference images" and "Run testing", for more information on the subject.
The title of the PR is "Performance optimizations", but the "Optimize performance of CanvasGraphics_paintInlineImageXObject function by not creating temporary images in while loop" commit isn't just an optimization, but actually alters the logic of the code. Most likely that this commit explains the regressions, and in my opinion it should be removed from the PR (https://github.com/mozilla/pdf.js/pull/8650#discussion_r127568469 explains perfectly well why the code looks like it currently does)!
Furthermore, I'm finding the "Optimize performance of PageViewport by reducing the number of calculations within the function" to make the code somewhat difficult to follow. I'll leave the actual review to @yurydelendik, but I cannot help wonder if needs to be this complicated?

I'm going to close this pull request because log2 is already fixed in #9256, the canvas graphics commit causes regressions and the page viewport changes are currently a bit hard to follow and needs more explanation as to why it works. If you want, you can revisit the page viewport changes in a new pull request, which will also make it easier to review. Thank you.


Expose federation status

If federation is enabled on the cluster being monitored, expose status of each federation upstream as a 0/1 value.

Hi Fawad,
Thanks a bunch for your contribution to the project, really appreciate it. I have merged the commits into master.
Abey


Add support for clear command

This is a low-priority ticket, but it'd be cool if the CLI supported typing the clear command; I often use it to clean-up the terminal window as I'm typing many commands, and I imagine it'd be useful for folks using the tutorial as well.

done!


Add interface column in the list of DHCP IPv4/IPv6 clients

Add interface column in the list of DHCP IPv4/IPv6 clients

Hi Frédéric, can you provide a bit more information for this request (which page to look at for example)? Thanks in advance. Regards, Ad

@AdSchellevis hi here's a nice mockup :p

@thelinuxfr thank you for clarifying this.


Counter for number of mystery boxes opened

🚀 Feature Proposal
If on BSO, if we could have a counter similar to that of clues, like after opening a TMB, it says "you've opened 5 TMBs" or similar etc, with a different one for each type of mystery box

duplicate of #567


Code [EA011] description has weird ﾒ character

Changed "Husband to wifeﾒs group" to "Husband to wife's group"

Thanks Sean!


Mixin Improvements: Override, super()

I would like added features for mixins to make them more inline with direct inheritance in terms if power and flexibility.

The ability to override mixed in fields and functions. Currently mixin functions take precedence over what are defined in the class including them, meaning you cannot override them.

The ability to call super() in an overridden mixed in function, to call the original implementation from the class it was from.

With these added features, mixins can be used more like proper interfaces with default implementations, and in some cases replace prototypical inheritance.

Fixed overriding in mixing, but do you have any idea to implement the super() function?

One idea is to keep a list of the class’ mixins. If super is called in a method, it can check the mixin list first before going through the parent class hiearchy.


legion: hang on ~8 nodes of lassen

We can change the title once we have more information about what's going on. I see the following hang on several applications -- here are stacks from some of the nodes:
Dump 1: https://pastebin.com/4TaKAz0w
Dump 2: https://pastebin.com/V0ckQEh0
Dump 3: https://pastebin.com/rbLtUF9E

The thing that stands out in each is that they are all waiting on a collective from a future map:
Thread 11 (Thread 0x20009ea8f8b0 (LWP 178175)):
#0  0x00002000000fe92c in __pthread_cond_wait (cond=0x20009ea889b0, mutex=0x191e9498) at pthread_cond_wait.c:153
#1  0x00000000127f9f8c in Realm::CondVar::wait (this=0x20009ea889a8) at /g/g15/yadav2/taco/legion/legion/runtime/realm/mutex.cc:231
#2  0x00000000127761cc in Realm::KernelThreadTaskScheduler::worker_sleep (this=0x191e9490, switch_to=0x2034cc0015b0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1428
#3  0x0000000012773ddc in Realm::ThreadedTaskScheduler::thread_blocking (this=0x191e9490, thread=0x191f5cf0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:941
#4  0x000000001259d730 in Realm::Thread::wait_for_condition<Realm::EventTriggeredCondition> (cond=..., poisoned=@0x20009ea8a198: false)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/threads.inl:218
#5  0x0000000012587ea4 in Realm::Event::wait_faultaware (this=0x20009ea8a4d8, poisoned=@0x20009ea8a198: false) at /g/g15/yadav2/taco/legion/legion/runtime/realm/event_impl.cc:266
#6  0x00000000125879a4 in Realm::Event::wait (this=0x20009ea8a4d8) at /g/g15/yadav2/taco/legion/legion/runtime/realm/event_impl.cc:214
#7  0x0000000011282a20 in Legion::Internal::LgEvent::wait (this=0x20009ea8a4d8) at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion_types.h:2741
#8  0x00000000121978a0 in Legion::Internal::AllGatherCollective<false>::perform_collective_wait (this=0x20009ea8a390, block=true)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion_replication.cc:11003
#9  0x0000000012176a04 in Legion::Internal::ShardCollective::perform_collective_sync (this=0x20009ea8a390, precondition=...)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion_replication.cc:10567
Python Exception <class 'gdb.error'> There is no member or method named _M_value_field.:
#10 0x000000001217fa20 in Legion::Internal::FutureNameExchange::exchange_future_names (this=0x20009ea8a390, futures=std::map with 2 elements)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion_replication.cc:13122
#11 0x00000000117c2718 in Legion::Internal::ReplFutureMapImpl::get_all_futures (this=0x2034cc03da50, others=std::map with 0 elements)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/runtime.cc:4035
#12 0x00000000117c2978 in Legion::Internal::ReplFutureMapImpl::wait_all_results (this=0x2034cc03da50, silence_warnings=false, warning_string=0x0)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/runtime.cc:4062
#13 0x0000000011267d9c in Legion::FutureMap::wait_all_results (this=0x20009ea8bb18, silence_warnings=false, warning_string=0x0)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion.cc:2543
#14 0x000000001113ba2c in placeLegionA (ctx=0x2034bc078220, runtime=0x1903ffc0, A=..., rpoc=<optimized out>, c=<optimized out>)
    at /g/g15/yadav2/taco/legion/solomonikMM/taco-generated.cu:119
#15 0x00000000111191ac in operator() (__closure=0x2034cc028930) at /g/g15/yadav2/taco/legion/solomonikMM/main.cpp:89
#16 std::_Function_handler<void(), top_level_task(const Legion::Task*, const std::vector<Legion::PhysicalRegion>&, Legion::Context, Legion::Runtime*)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:297
#17 0x0000000011128ed8 in operator() (this=0x20009ea8cdf0) at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:260
#18 benchmark(Legion::Internal::TaskContext*, Legion::Runtime*, std::vector<unsigned long, std::allocator<unsigned long> >&, std::function<void ()>) (ctx=0x2034bc078220,
    runtime=0x1903ffc0, times=std::vector of length 0, capacity 0, f=...) at /g/g15/yadav2/taco/legion/src/legion_utils.cpp:35
#19 0x000000001111d530 in top_level_task (ctx=<optimized out>, runtime=<optimized out>, regions=..., task=<optimized out>)
    at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:87
---Type <return> to continue, or q <return> to quit---
#20 0x000000001111dd5c in Legion::LegionTaskWrapper::legion_task_wrapper<&(top_level_task(Legion::Task const*, std::vector<Legion::PhysicalRegion, std::allocator<Legion::PhysicalRegion> > const&, Legion::Internal::TaskContext*, Legion::Runtime*))> (args=<optimized out>, arglen=<optimized out>, userdata=<optimized out>, userlen=<optimized out>, p=...)
    at /g/g15/yadav2/taco/legion/legion/cmake-install/include/legion/legion.inl:22539
#21 0x00000000126dc9b8 in Realm::LocalTaskProcessor::execute_task (this=0x193a4160, func_id=16, task_args=...) at /g/g15/yadav2/taco/legion/legion/runtime/realm/proc_impl.cc:1150
#22 0x0000000012770e7c in Realm::Task::execute_on_processor (this=0x2034bc07f260, p=...) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:306
#23 0x0000000012775eec in Realm::KernelThreadTaskScheduler::execute_task (this=0x191e9490, task=0x2034bc07f260) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1380
#24 0x0000000012fe2940 in Realm::OpenMPTaskScheduler<Realm::KernelThreadTaskScheduler>::execute_task (this=0x191e9490, task=0x2034bc07f260)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/openmp/openmp_module.cc:125
#25 0x0000000012774aac in Realm::ThreadedTaskScheduler::scheduler_loop (this=0x191e9490) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1127
#26 0x000000001277500c in Realm::ThreadedTaskScheduler::scheduler_loop_wlock (this=0x191e9490) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1231
#27 0x000000001277ffd0 in Realm::Thread::thread_entry_wrapper<Realm::ThreadedTaskScheduler, &Realm::ThreadedTaskScheduler::scheduler_loop_wlock> (obj=0x191e9490)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/threads.inl:97
#28 0x00000000127930ec in Realm::KernelThread::pthread_entry (data=0x191f5cf0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/threads.cc:774
#29 0x00002000000f8cd4 in start_thread (arg=0x20009ea8f8b0) at pthread_create.c:309
#30 0x0000200006117e14 in clone () at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:104

I haven't had any luck reproducing this on Sapling, but I'm happy to gdb into a stuck process and give you the necessary information.
Based on looking into this with Sean, we believe that the bug is a legion problem, as realm is showing that each node is waiting on an event that hasn't been triggered yet.

It appears to not be stuck with those arguments (though I imagine they have performance impacts?).

If it still hangs then make me a reproducer on sapling.

As I mentioned in the issue, I haven't been able to reproduce the bug on sapling.

I take that back -- it appears to get through all of my program and then is stuck on shutdown with -lg:safe_ctrlrepl 1 -lg:inorder. There doesn't appear to be anything interesting in any of the stacks of nodes I randomly sampled as well. This might be an unrelated problem.

I added a missing check to the safe control replication checks related to this. Pull and try again with the safe control replication checks after rebuilding.
If they still pass then attach a debugger to each of the shards and print out the value of collective_index on this line of frame 11 in the interesting threads:
https://gitlab.com/StanfordLegion/legion/-/blob/control_replication/runtime/legion/runtime.cc?expanded=true#L4040

The new checks didn't fire and all nodes have the same collective_index = 1140. I think I found some interesting details though:

The code succeeds with -lg:inorder and hangs without.
One of the nodes has a slightly different stack than all of the others (like the ones linked in the pastes):

#0  0x00002000000fe92c in __pthread_cond_wait (cond=0x20009efba1a8, mutex=0x170197d8) at pthread_cond_wait.c:153
#1  0x0000000010e482c0 in Realm::CondVar::wait (this=<error reading variable: value has been optimized out>) at /g/g15/yadav2/taco/legion/legion/runtime/realm/mutex.cc:231
#2  0x0000000010dfc0b8 in Realm::KernelThreadTaskScheduler::worker_sleep (this=0x170197d0, switch_to=0x2034d0001dd0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1428
#3  0x0000000010dfd758 in Realm::ThreadedTaskScheduler::thread_blocking (this=0x170197d0, thread=<optimized out>) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:941
#4  0x0000000010d32874 in Realm::Thread::wait_for_condition<Realm::EventTriggeredCondition> (cond=..., poisoned=@0x20009efbaad8: false)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/event_impl.cc:168
#5  0x0000000010d20b0c in Realm::Event::wait_faultaware (this=0x2034d0075fe0, poisoned=@0x20009efbaad8: false) at /g/g15/yadav2/taco/legion/legion/runtime/realm/event_impl.cc:266
#6  0x0000000010d20d0c in Realm::Event::wait (this=<optimized out>) at /g/g15/yadav2/taco/legion/legion/runtime/realm/event_impl.cc:214
#7  0x000000001047d4e4 in Legion::Internal::LgEvent::wait (this=<optimized out>) at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion_types.h:2757
#8  0x00000000106a40cc in Legion::Internal::ReplFutureMapImpl::get_all_futures (this=0x2034d0075d40, others=std::map with 0 elements)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/runtime.cc:4025
#9  0x000000001064c964 in Legion::Internal::ReplFutureMapImpl::wait_all_results (this=0x2034d0075d40, silence_warnings=<optimized out>, warning_string=<optimized out>)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/runtime.cc:4061
#10 0x00000000103d3bd8 in Legion::FutureMap::wait_all_results (this=<optimized out>, silence_warnings=<optimized out>, warning_string=<optimized out>)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/legion.cc:2565
#11 0x0000000010367600 in placeLegionA (ctx=0x2034c4000900, runtime=0x16fb3eb0, A=..., rpoc=<optimized out>, c=<optimized out>)
    at /g/g15/yadav2/taco/legion/solomonikMM/taco-generated.cu:119
#12 0x000000001033fdcc in operator() (__closure=0x2034d006b550) at /g/g15/yadav2/taco/legion/solomonikMM/main.cpp:89
#13 std::_Function_handler<void(), top_level_task(const Legion::Task*, const std::vector<Legion::PhysicalRegion>&, Legion::Context, Legion::Runtime*)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:297
#14 0x00000000103509f8 in operator() (this=0x20009efbd5d0) at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:260
#15 benchmark(Legion::Internal::TaskContext*, Legion::Runtime*, std::vector<unsigned long, std::allocator<unsigned long> >&, std::function<void ()>) (ctx=0x2034c4000900,
    runtime=0x16fb3eb0, times=std::vector of length 5, capacity 8 = {...}, f=...) at /g/g15/yadav2/taco/legion/src/legion_utils.cpp:35
#16 0x0000000010344180 in top_level_task (ctx=<optimized out>, runtime=<optimized out>, regions=..., task=<optimized out>)
    at /usr/tce/packages/gcc/gcc-8.3.1/rh/usr/include/c++/8/bits/std_function.h:87
#17 0x00000000103449ac in Legion::LegionTaskWrapper::legion_task_wrapper<&(top_level_task(Legion::Task const*, std::vector<Legion::PhysicalRegion, std::allocator<Legion::PhysicalRegion> > const&, Legion::Internal::TaskContext*, Legion::Runtime*))> (args=<optimized out>, arglen=<optimized out>, userdata=<optimized out>, userlen=<optimized out>, p=...)
    at /g/g15/yadav2/taco/legion/legion/cmake-install/include/legion/legion.inl:22541
#18 0x0000000010dadb3c in Realm::LocalTaskProcessor::execute_task (this=0x171d7e10, func_id=<optimized out>, task_args=...)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/bytearray.inl:150
#19 0x0000000010dfc444 in Realm::Task::execute_on_processor (this=0x2034c40224d0, p=...) at /g/g15/yadav2/taco/legion/legion/runtime/realm/runtime_impl.h:378
#20 0x0000000010dfc5a4 in Realm::KernelThreadTaskScheduler::execute_task (this=<optimized out>, task=<optimized out>) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1380
#21 0x000000001138d4fc in Realm::OpenMPTaskScheduler<Realm::KernelThreadTaskScheduler>::execute_task (this=0x170197d0, task=0x2034c40224d0)
    at /g/g15/yadav2/taco/legion/legion/runtime/realm/openmp/openmp_module.cc:122
#22 0x0000000010dff370 in Realm::ThreadedTaskScheduler::scheduler_loop (this=0x170197d0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1125
#23 0x0000000010dff928 in Realm::ThreadedTaskScheduler::scheduler_loop_wlock (this=0x170197d0) at /g/g15/yadav2/taco/legion/legion/runtime/realm/tasks.cc:1231
#24 0x0000000010e02b34 in Realm::KernelThread::pthread_entry (data=0x17026030) at /g/g15/yadav2/taco/legion/legion/runtime/realm/threads.cc:774
#25 0x00002000000f8cd4 in start_thread (arg=0x20009efbf8b0) at pthread_create.c:309
#26 0x0000200006627e14 in clone () at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:104
(gdb)

While all of the other nodes are doing a collective sync, this one is just waiting on an event, which is probably the deadlock.

Some information from this stack:
(gdb) f 9
#9  0x000000001064c964 in Legion::Internal::ReplFutureMapImpl::wait_all_results (this=0x2034d0075d40, silence_warnings=<optimized out>, warning_string=<optimized out>)
    at /g/g15/yadav2/taco/legion/legion/runtime/legion/runtime.cc:4061
4061	      get_all_futures(dummy_others);
(gdb) info locals
dummy_others = std::map with 0 elements

(gdb) info locals
mutator = {<Legion::Internal::ReferenceMutator> = {_vptr.ReferenceMutator = 0x0}, mutation_effects = <error reading variable: Cannot access memory at address 0x4b000000a3>}
f_lock = {local_lock = @0x200000000000, previous = 0x0, exclusive = false, held = 165}

And that's why you need to look at the stack traces on all the nodes and not just some of them. This backtrace all but guarantees that one or more tasks didn't run on this shard for this particular index space launch.
Run again with -ll:defalloc 0.

And that's why you need to look at the stack traces on all the nodes and not just some of them.

Is there a more efficient way of doing this than ssh-ing into each node and attaching to the process with gdb? This is sort of annoying at 8 nodes, and prohibitive at 32 nodes (where I see a similar hang but on a different application).

This backtrace all but guarantees that one or more tasks didn't run on this shard for this particular index space launch.

That's true for this application, but isn't true for another application that I think is hanging the same way.

Run again with -ll:defalloc 0.

This causes my program to OOM (failed instance allocations in the mapper). When I my application doesn't hang, it runs to completion without OOMs.

And that's why you need to look at the stack traces on all the nodes and not just some of them.

Is there a more efficient way of doing this than ssh-ing into each node and attaching to the process with gdb? This is sort of annoying at 8 nodes, and prohibitive at 32 nodes (where I see a similar hang but on a different application).

Try something like:
for JOB_COMPUTE_HOST in ...; do
  ssh $JOB_COMPUTE_HOST <legion-dir>/tools/print_backtraces.sh <executable-name> > $JOB_COMPUTE_HOST.txt
done

@manopapad said exactly what I was going to say for capturing the backtraces. There are very few things that we'll ask you to do that don't already have a tool.

That's true for this application, but isn't true for another application that I think is hanging the same way.

Then you'll need to find a reproducer for that code that hangs with -ll:defalloc 0

This causes my application to OOM (failed instance allocations in the mapper). When my application doesn't hang, it runs to completion without OOMs (when I don't use -ll:defalloc 0).

This is a known problem how deferred buffers are allocated today: they don't guarantee deadlock freedom with deferred allocation. There is a pending but incomplete fix in the newinsts branch. The group has instructed me to put all my Legion development time towards collective instances and control replication for the time being. If you want to change those priorities then you'll need to bring it up in a Legion meeting.

I can try to get a reproducer with -ll:defalloc 0, but my code doesn't used any deferred buffers.

I have a reproducer on sapling that you can use now.  You should see a print every ~10 seconds. If the prints stop showing up then it's hanging.

There are other forms of deferred allocation other than deferred buffers/values. If you want to convince me that there is a hang that doesn't involve deferred allocation, then it needs to reproduce with the -ll:defalloc 0 flag, and this reproducer can't do that. Alternatively, you can find a cycle in the Realm event graph that doesn't involve deferred allocation, but I suspect that such a cycle does not exist.

@streichler also said that he was planning on adding support for detecting cycles to Realm's hang detection mechanism, so getting Realm's hang detection to tell us (with Legion Spy logging) what the event cycle is involving deferred allocation would be instructive, but I'm 99% positive that it's going to involve deferred allocation.

I'm not successful in getting a reproduction with -ll:defalloc 0 -- everything either succeeds on a smaller problem size or OOMs on problem sizes close to my target. What are my options here?

Convince the group to prioritize work on the newinsts branch
Is there something that I could "use less of" or stop using in my code to make the hang less likely?

I need these applications to complete so that I have a full set of experiments for the PLDI deadline in mid-november. If a timeline for work on newinsts wouldn't complete by then even with priority, then I'm a bit stuck for what to do.

It's possible that there is a faster solution to the problem, but we can't know that until we've characterized the exact nature of the deferred allocations that are causing the problem. Without that we can't say which approach is the best. I would try to modify the Realm cycle detection mechanism to add edges through deferred allocations so that it can "see" those kinds of cycles too.

I would try to modify the Realm cycle detection mechanism to add edges through deferred allocations so that it can "see" those kinds of cycles too.

I can give this a shot if you give me some pointers @streichler, or if you think it's more efficient for you to just do I can wait on it.

@lightsighter it will not be helpful (for this issue) to teach realm to find cycles through deferred allocations.  Although there are deferred instance destructions pending in every one of the event waiter dumps @rohany has captured, in all cases the destructions are waiting on user events that have been created by legion but not triggered (i.e. neither deferred nor immediate), so there is no loop involving deferred allocations in the realm operation graph itself.

If figuring out the source of the deferred allocation hangs comes back to me, it's going to wait until after GTC, Supercomputing, and the PLDI deadline have all passed.

I was able to mostly work around this by backpressuring my task's mapping so that the working set never exceeded the amount of memory in the GPU. I ran into the unable to find distributed collectible bug again at 128 nodes, but I was able to run my app for fewer iterations to not hit it.

Run with -ll:force_kthreads and capture backtraces on every single node when you crash with the unable to find distributed collectable bug. Report the full error message for the crash along with the log files.

I can do that. Is there a way that I can get backtraces sent to a log file "automatically"? 128 node jobs on lassen take many hours to come back, often executing overnight. I can't guarantee that I'll be around to be able to manually run the script to gdb into each node to collect the backtraces.

along with the log files.

Is there a particular logger that you want me to turn on?

Is there a way that I can get backtraces sent to a log file "automatically"?

Would something like this work?
jsrun <appname> &
sleep <job-timelimit - 1h>
for HOST in $LSB_HOSTS; do
  ssh $HOST <legion-dir>/tools/print_backtraces.sh <appname> > $HOST.txt
done
wait

That's a good idea. I'll try that.

@rohany Did we sort this one out?

There were a few points in the issue I think:

The original hang due to deferred allocations -- I worked around this by backpressuring task execution and using -defalloc 0. But the hang without defalloc is still there.
The distributed collectible error -- I don't know if Mike got a chance to look at those logs, but this bug has popped up in different places, so this likely isn't the issue to be tracking it.


Check domain config instead of all domains

You are checking something different then displaying. When no subdomains are present in the config, this gives a warning otherwise.

Thanks!


Report number of hits, not just pages, for scroll queries

It is useful to know how many hits that were retrieved by a scroll query, not just the number of pages, as this allows analysis of the efficiency of various page sizes more easily.

This can be solved pretty easily and is completely local to Query#scroll_query(). We just need to keep track of the results count and return a dict with the meta-data instead of a tuple.


feat/irec connection ui

In this PR:

Added Page with form to allow user set connection to I-REC API
Refactored origin-ui-organization-data folder structure to match with all other packages (fetching / handlers)
Merged latest master

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.2 out of 3 committers have signed the CLA.:white_check_mark: ioncreature:white_check_mark: alexworker23:x: github-actions[bot]You have signed the CLA already but the status is still pending? Let us recheck it.


⚠ ClamAV has degraded performance

In 0349462, ClamAV (https://spamassassin.apache.org/) experienced degraded performance:

HTTP code: 200
Response time: 72 ms

Resolved: ClamAV performance has improved in 2a0ea2d after 9 minutes.


Embedded extensions not respecting the "Hide" login prompt setting

Describe the issue
Since upgrading servers to 2021.4.2, our Extensions have been displaying the Login modal, despite having all of the User Prompts set to “Hide” in the Extension settings.
Version information:

Tableau version:  Tableau Server 2021.4.2
OS: Tableau Server is on MIcrosoft Windows 2016
Browser: Tested on Chrome, Safari, and Mozilla
Version: Chrome 97.0.4692.99

A clear and concise description of what you expected to happen.
For reference, this is pertinent to our case. Our Tableau Server is hosted on a different domain than where the Extension API is embedded:
https://kb.tableau.com/articles/Issue/embedded-views-fail-to-load-after-updating-to-chrome-80
However, I just tested and this is not limited to Chrome. And obviously, we cannot micromanage Chrome on behalf of our users, or even enforce that they use Chrome to begin with.
Our infrastructure team have requirements that make them opposed to enabling SSL as that article suggests. That is also not a trivial thing to test. If I could have guarantees that enabling SSL would resolve this problem, I could make the case to the infrastructure team. I would love to hear more about whether or not that is an accurate diagnosis!
If possible, attach a workbook with sample data add/or screenshots to help explain your problem.
I can share the Extension privately if that would be helpful. Here are screenshots:

@bcranteverfi : Thanks for your report. We have duplicated this in-house, and it is a high priority to fix immediately.
John

Logged internally as #1369679

@bcranteverfi
It turns out that on the same day as you reported, there was another report about a permissions dialog. We mistakenly combined the two issues into a single one. When you say the "login model" are you referring to the extension permission dialog, or logging into Tableau Server? It appears that your second attachment is an embedded Tableau scenario. Is this where you are getting the login? If so, that is unrelated to your dashboard extensions settings. Below is what the extension permission prompt looks like. This is shown or hidden based on the settings you highlight above.
Thank you, and sorry for the confusion.
John

@johnDance Hey John, sorry for slow reply, I just moved to Phoenix and have been out this week.
Circling back here, my mistake on the premise of the issue. I have only ever seen the "Allow Extension" modal that you posted in Tableau Desktop, not the embedded Extension, so the "Hide" setting is operating as expected on the Server.
I will reach out to the Embedded team for further guidance.
Thanks again for your help!


bug found multiple images (embedBlock)

In embedblock images are not displayed when posting more than 1 of the same link.
http://jsfiddle.net/50a7kwp5/35/

duplicates will not be visible. thats intentional.

i know. but 2 links are the same image. the 3e is differten link. the 3e image is not showing.

Updated the fiddle http://jsfiddle.net/50a7kwp5/36/

this is a duplicate of #28 . It has already been solved in the latest commit.

But that's the weird thing: i am using the latest build..
http://jsfiddle.net/50a7kwp5/37/

Please use codepen instead of jsfiddle :)

All these problems arise because you are replacing newline with
 so the string presented to embedJS is @(IIT Roorkee)
same:
http://static1.ad.nl/static/photo/2015/10/14/5/20150419232616/media_l_2922875.jpg
http://static1.ad.nl/static/photo/2015/10/14/5/20150419232616/media_l_2922875.jpg
new:
http://static2.ad.nl/static/photo/2015/16/13/2/20150419223816/media_l_2923622.jpg
 and all appears as a single word.

here's the blunder:
by default imageEmbed is set to false . Set it to true.

fixed

Yess! You did it !


Refresh can return an empty dictionary if called to quickly

Update the Refresh to handle empty data where the dictionary is completely blank

Fixed


Add minimum response time to every p2p call

We want to avoid having a timing attack where an invoker can figure out certain information based on the time a request takes to perform.
We should apply this to:

Every p2p endpoint
wether the data is available or not

Take some time to research on current average response times for above scenario to find the correct response delay.
Add this timing delay handling in the interceptor, so it is applied to all requests.

Fixed with #1013


Release Google.Cloud.ArtifactRegistry.V1Beta2 version 2.0.0-beta04

Changes in this release:
New features

Change netstandard2.1 target to netstandard2.0 (commit 82bea85)

Created release for Google.Cloud.ArtifactRegistry.V1Beta2-2.0.0-beta04


Update convert-user-mailbox-to-shared-mailbox.md

A Global Admin doesn’t have any privilege on-premises, so this information doesn’t make sense. I guess that the point would be covering on how to convert the mailbox in Exchange On-Prem using Powershell. If so, the best thing is to move this Note to the “Hybrid Session” at the end. Another thing is that the command is wrong.

@SignorelliDenis Can you please fix the conflict so that I can get this reviewed?

@SignorelliDenis Can you please fix the conflict so that I can get this reviewed?

Done!

@dianef77 - can you please assist to approve and merge the content for this PR?
Thank you.


Handle non-existing address consistently in getAccount

getAccount(query: BcpAccountQuery): Promise<BcpQueryEnvelope<BcpAccount>> should have a consistent way to report non-existing addresses. Those errors must be standardized across all implementations and differentiable from programming erros like

Cannot read property 'balance' of undefined

or network errors. I need this to implement address discovers in HD wallets.
There are two possible ways:

Non-existing addresses are promise rejections: we need an error we can catch and classify
Non-existing addresses are encoded as a success in BcpAccount, e.g. balance array empty

Option 3: change interface to
getAccount(query: BcpAccountQuery): Promise<BcpQueryEnvelope<BcpAccount> | undefined>

where undefined means no error but account is not available.

This is already covered and tested on BNS: https://github.com/iov-one/iov-core/blob/da17624/packages/iov-bns/src/client.spec.ts#L161 There we use an empty data array in BcpQueryEnvelope (i.e. no accounts). I think this is good. Will document this, make the test more explicit and fix lisk accordingly.


Error on automatic Scratch Org generation

node:internal/modules/cjs/loader:93[6](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:7)`
  throw err;
  ^

Error: Cannot find module 'xml2js'
Require stack:
- /home/runner/work/rd-apex-case-correspondence-check/rd-apex-case-correspondence-check/.github/scripts/buildReadme.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:[7](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:8)7[8](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:9):27)
    at Module.require (node:internal/modules/cjs/loader:1005:1[9](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:10))
    at require (node:internal/modules/cjs/helpers:[10](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:11)2:18)
    at Object.<anonymous> (/home/runner/work/rd-apex-case-correspondence-check/rd-apex-case-correspondence-check/.github/scripts/buildReadme.js:25:21)
    at Module._compile (node:internal/modules/cjs/loader:[11](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:12)05:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:[12](https://github.com/hwconsult/rd-apex-case-correspondence-check/runs/6654219607?check_suite_focus=true#step:11:13))
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/runner/work/rd-apex-case-correspondence-check/rd-apex-case-correspondence-check/.github/scripts/buildReadme.js'
  ]
}
Error: Process completed with exit code 1.

Seems like the Workflow Action that generates the README installs npm updates (which works), but the implicit README update in the scratch org creation Workflow does not.

I helped myself for now by adding lines for installing npm dependencies right after source check out in the scratch org create workflow, but I don't know if this is a viable solution for all workflows.

I think this is a viable solution for the issue. Thanks for your great help @birgerwollburg.

🤙


Find observable transits and plot airmass curves for them!

Take an exoatlas transiting exoplanet population and figure out what transits can be observed from a particular observatory on a particular night.
@miricaradonna (details to follow!)

@MiriCaradonna , we talked about this together as being two different functions. We might even want to split it down further into more functions, too, but here's the basic idea:
find_observable_transits() (or some better name) could take as inputs:

an Exoplanet Atlas Population and/or a planning table generated by Population.create_planning_table(), either of which contain the names of planets, their periods, their times of mid-transit, and their transit durations
an observatory or observing location, possibly/probably in the form of an astropy EarthLocation from which we'll be observing, necessary to know when what RA and Dec are visible at any particular moment
a time/date range, possibly/probably in the form of astropy Time objects, indicating the range of times we should explore for possible transit events. Maybe it's two times for a start and finish, or maybe it's one time for a start and then a number of days after that to include
a maximum airmass or minimum altitude or maximum zenith angle (however you want to think about it) beyond which we don't want to observe
what kind of twilight (see here we need to avoid. For many bright targets, "12 degree = nautical" twilight is probably a good default?

and it could return

a list or table of transit events, somehow capturing (at least) the ingress/midpoint/egress of each observable transit. Depending on how we want this function to interact with the next one, we might want more or less information contained in these outputs.

plot_airmass_curves() (or some better name) could then take as inputs

the outputs from the previous function

and it could produce plots of airmass curves for the transits, indicating...

the airmass/altitude of the target throughout the night
the times of twilight (dusk and dawn)
the transit ingress, midpoint, and egress

It might be that in both of these functions we want to focus on either one planet at a time or on one night at time. As you're experimenting with astroplan for this, let's talk about what makes sense!

Let's revise the suggestion here to more closely match what @MiriCaradonna figured out makes the most sense, that find_transits should just find the available transits for one planet.
Talking about it just now, we think that it should return an astropy.table.Table object containing one row for each observable transit within a given time range for just one planet. Then, if we wanted to find all the transits observable for all planets in a population, we could have another function find_transits_for_all_planets (or some such) that effectively just loops over planets and calls find_transits for each. Maybe it could use astropy.table.vstack to stack a list of individual planet tables into one jumbo super table?

Awesome, @MiriCaradonna , now that I looked I see your https://github.com/zkbt/exoatlas/tree/observable_transits branch. It'd be amazing if you could please open a Pull Request from it, where we can look at the code changes and make any revisions we need and then ultimately merge it into the main code!


bugfix: properties tab

Fixes an issue where the application where completing the Properties tab causes saving a service to fail.

Hi @mmoayyed, this fix stopped working with commits #196


I want to add floyd's cycle detection algorithm for linked list

I want to add floyd's cycle detection algorithm for linked list.
Is your feature request related to a problem? Please describe.
No.
Describe the solution you'd like
Implement floyd's cycle detection algorithm using fast and slow pointer.
Additional context
More context from wikipeida
Languages

[x] C

@listout assigned


Remove unused iOS simulators from devicelab Xcode

These simulators pollute test logs during xcrun xcdevice list, and may cause the actual device to be not found within timeout limits (see, e.g., https://github.com/flutter/flutter/issues/67753).

I've removed dozens of simulators from Mac11. Let's see if it helps #67753.

I took a look at that test, seems like the device is taking longer to get discovered and now it needs 20 seconds to successfully discover it.

time xcrun xcdevice list gave me ~5 seconds when I tried it on Mac 11. Anyway, the reduced simulators seem to make #67753 happen less, but there's still flakiness. Hence increasing the timeout seems to be necessary, and reducing simulators would be to mainly reduce log noise and other flakiness.

This won't be needed as soon as we migrate devicelab to luci.


Wireframe V2

I would like to discuss merging feature/v2 branch to master .
First I would like if you could go over the proposed changes to settle all creative differences.

#6

I've added current files and templates as "deprecated" in this proposal.

Closing this one, since PR is merged.


Add Angrylion RDP article

I'm not certain emudev.org is the best place for this, but I've tried to better document some aspects of the N64 RDP by adding comments to Angrylion's RDP plugin, which should hopefully make it easier to understand. There are still some areas I'm not entirely clear on, and likely gaps that I haven't noticed as well, so comments, corrections, and additions would be much appreciated.

The entire website would benefit from higher width, I think.
Especially on this merged post some code sections aren't readable without having to horizontally scroll.


[Minor] [v2] Deep-linking and a compliment

Deep-linking
Not sure if a bug or intentional design, but in this screenshot the first 3 boxes point to the same link, instead of, e.g., making "Understand the tooling" point to https://www.typescriptlang.org/docs/handbook/tsconfig-json.html

A compliment
Whoever has the designed the screenshot above is a genius!  ... I indeed came for the tooling section, and the 2 or 3 other sections concisely compress my goals when accessing the site. Also, the release box is just 🚀
So, yes, feel free to close if the 3 boxes linking to the same page is intentional.

Thanks!
Yeah, I spent some time having a think about what the best call to action could be once you've got a general sense of where you might want to go once you might have been persuaded that TS is worth your time.
It took 4 designs to get to that quarterly releases section, even now it's quite "representational" on timelines but I think it shows the idea well!


Hue Ambiance and Full Color Lights Together - Color Not Set

What is happening?
Hey there - excellent repo! Great functionality. I’m running into an issue though when I have a Zigbee group of Hue lights where some are full color and some are ambiance (ie can only adjust color temperature). Here’s what happens (I’m using the standard Hue scenes of Relax and Concentrate for these tests):
If the Zigbee group has only full color lights, or only has ambiance lights:

Applying the single preset works fine - correct color and brightness (all light same color and brightness)

If the Zigbee group is a mixture of full color and ambiance lights:

The full color lights get the scene preset applied correctly - the correct color and brightness
The ambiance lights only adjust their brightness (correctly) but the color doesn’t update
So the end result is some lights are different colors

If you do the same but with an HA group instead, everything works fine.
I’ll add that if you directly adjust the color temperature of the Zigbee group with a mixture of full color and ambiance bulbs, it works correctly…so this doesn’t seem to be a Zigbee group limitation…
Thoughts? Thanks!
What should be happening?
Applying the preset to the combined Zigbee group of ambiance and full color lights should have the same impact as applying the preset (simultaneously) to two Zigbee groups (one with full color and one with ambiance bulbs), which correctly sets the right color and brightness to all lights.
Further, this correct behavior is also achievable using the “light.turn_on” setting the color temperature on the combined Zigbee group, so there isn’t something fundamentally unachievable due to the Zigbee group
Reflection 1
Objective Input
Is this issue tracker the right place for this topic?
I believe so, as the functionality seems achievable with hacks (eg separating everything into two groups), so I’m curious if the correct behavior can be achieved in the code?
Please enter the scene_preset version you're using
2.2.1
Please enter the Home Assistant version you're using
2024.12.5
Details about your setup
ZHA. Hue bulbs - some are full color and some are ambiance. Mixture of A19 bulbs and recessed lights.
Reflection 2
Mixture of ambiance and full color lights. But this seems fairly common.
Time and Effort

[X] Yes, I did my homework.
[X] I understand that my issue will be deleted with no further comment if I did not do my homework.

One slight additional point - as for why to use Zigbee group instead of HA group - I have Zigbee light switches that are bound to the Zigbee groups, which I imagine is not uncommon.
Happy to dig into the code if you can point me to where would be helpful (unless I've configured something wrong here...)
Thanks again - and happy new year!!


MGMT-16109: Remove api folder from ocm and use the api folder from common

Related to https://issues.redhat.com/browse/MGMT-16109

@ammont82 the i9n tests failed, here's the failing test. Reason: TypeError: win.__app__.OCM.setAuthInterceptor is not a function. This is due to the refactor made in /ui-lib/lib/ocm/index.ts.
In order to fix that the code should access the setAuthInterceptor function from OCM.Api, like this: win.__app__.OCM.Api.setAuthInterceptor.


VS2017 typescript breakpoints not working with the Angular Microsoft.DotNet.Web.Spa.ProjectTemplates::2.0.0 template

When I create a new project using the Microsoft.DotNet.Web.Spa.ProjectTemplates::2.0.0 Angular template, debugging of typescript in VS2017 is not working.
I can set a breakpoint in Chrome and that works OK, but I can't set a typescript breakpoint in VS2017. The previous version of this template (1.0.417) does allow debugging and the setting of typescript breakpoints in my current configuration of VS2017.
If I include a debugger; statement in the typescript, it does break in VS2017, but no code is displayed.
The typescript sources are visible in the Chrome Debugger under webpack:// but are not shown in VS2017.
Can anyone point me in the right direction to investigate this, please?

@JasonBro - this sounds like an issue with VS's debugging support, so I recommend logging an issue using VS's built-in "Send feedback / Report a problem" button in the upper-right corner of the VS window. That will ensure that the issue gets routed to the right people in VS and you'll be able to follow the issue's progress there. Unfortunately I don't think there's anything we can do here from within this specific project template.

I think it's more to do with the configuration of the typescript files when used with webpack. I think the map files are not being produced in the way that VS2017 likes them. It's difficult to argue that it's a problem with VS2017's debugging when debugging worked with the previous version of the SPA template, so I would urge you to re-open this and have another look at the problem.
Unfortunately my knowledge of webpack and the typescript configuration is somewhat limited, but I'll play around with it until I find a solution.

Understood, thanks for the clarification.
@SteveSandersonMS  - I imagine you have a better sense of these types of issues. If you had to guess, would you think it's more of a VS issue, or we did something odd in the templates?

It's a duplicate of this: https://github.com/aspnet/JavaScriptServices/issues/1474
Summary: VS and Angular CLI aren't aware of each others' source map formats.

It's difficult to argue that it's a problem with VS2017's debugging when debugging worked with the previous version of the SPA template

It's specifically because we move to the Angular CLI-based template and hence don't control the source maps format.

@SteveSandersonMS

It's specifically because we move to the Angular CLI-based template and hence don't control the source maps format.

Sorry if I got that wrong, but to me it sounds a bit like "there's nothing we can do about it". Is this issue something you will resolve before releasing .NET Core 2.1 or is it not on the backlog at all?
Don't mean to sound rude or anything, just a quick clarification would be nice.

@Stefan-Ilic The compatibility between Angular CLI and VS is a matter for Angular CLI and VS, not the template in this repo. So I hope they will make this work, but I don't have any inside information about that. It's not something we control in this repo.

Closing as dup.


⚠ Bofrost.es has degraded performance

In d4a8b72, Bofrost.es (https://www.bofrost.es) experienced degraded performance:

HTTP code: 200
Response time: 1487 ms

Resolved: Bofrost.es performance has improved in 2dc2952 after 7 minutes.


Facet router

Can you please split package.json/yarn.lock changes to a separate commit within this PR?

@mareklibra rebased, no changes in yarn.lock


[topgen/tlgen] Multi-tiered Xbar

This PR is to support multiple level of crossbars in top_earlgrey.
Now, top_earlgrey can have multiple crossbars connecting each other.
The tool can do followings :

Detect if the host/device port is connected to another xbar with xbar:true field.
Gather the device port address if the device port is to another xbar.
Simplify the device port base_addr and size_byte if there's no other device in the gap.
for instance, if device xbar has address 0x4000_0000 +0x1000 and 0x4001_0000 + 0x1000 and the current xbar has no device having the range between 0x4000_1000 and 0x4001_0000, the address for the device port becomes 0x4000_0000 + 0x1_1000 .
Multiple address range is supported for a device port

it would be easier to build testbench if we treat each xbar as an IP
hw/top_earlgrey/xbar_main/
hw/top_earlgrey/xbar_perif

OK, the Unknown assertion comes from prim_fifo_sync inside RV_CORE_IBEX. the Rdata should be ignored when the response is for Put request. After commenting out (temporary), the chip_sanity test passed.

Now, xbars are placed into each directory:

hw/top_earlgrey/ip/xbar_main
hw/top_earlgrey/ip/xbar_peri

On Thu, Dec 19, 2019 at 06:27:00PM -0800, weicaiyang wrote:

it would be easier to build testbench if we treat each xbar as an IP
hw/top_earlgrey/xbar_main/
hw/top_earlgrey/xbar_perif
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/lowRISC/opentitan/pull/1236#issuecomment-567760356

Thanks for doing this Eunchan, I'll have a look at this later this week.
On Mon, Dec 23, 2019, 08:21 Eunchan Kim<EMAIL_ADDRESS>wrote:

Now, xbars are placed into each directory:

hw/top_earlgrey/ip/xbar_main
hw/top_earlgrey/ip/xbar_peri

On Thu, Dec 19, 2019 at 06:27:00PM -0800, weicaiyang wrote:

it would be easier to build testbench if we treat each xbar as an IP
hw/top_earlgrey/xbar_main/
hw/top_earlgrey/xbar_perif
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/lowRISC/opentitan/pull/1236#issuecomment-567760356

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lowRISC/opentitan/pull/1236?email_source=notifications&email_token=AAH2RSTQ5GGLAC45ZF4TZLTQ2DQPXA5CNFSM4J5UQ5MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHROEWY#issuecomment-568517211,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAH2RSQI4DKSAGEQDN23EFTQ2DQPXANCNFSM4J5UQ5MA
.

Now, xbars are placed into each directory: - hw/top_earlgrey/ip/xbar_main - hw/top_earlgrey/ip/xbar_peri

Thanks @eunchan

LGTM. Do you think there's any need to try this on FPGA before merging?
I'm thinking no...since none of the base addresses really changed, so there should be no impact on software..

At least, I've tested on Verilator (hello_world). I will try FPGA .

OK. FPGA works. I will merge the PR soon.

thanks for chiming in! i think let's discuss this in person again with a
few others. My change of heart is mostly coming from the fact that I view
fifo as a storage element, and my general feeling is that for storage
elements the 'X'-ness doesn't really make sense directly on the storage
element, but more on the protocol wrapper itself.
I can easily be convinced otherwise though :) So i guess that does mean I
would lean towards a parameterized option as you suggest. But maybe we
should discuss first.  I dont think we should add an option if there's no
need for it.
On Thu, Jan 2, 2020 at 12:37 PM Michael Schaffner<EMAIL_ADDRESS>wrote:

@msfschaffner commented on this pull request.
In hw/ip/tlul/rtl/tlul_fifo_sync.sv
https://github.com/lowRISC/opentitan/pull/1236#discussion_r362620973:

@@ -72,7 +72,7 @@ module tlul_fifo_sync #(
tl_d_i.d_size  ,
tl_d_i.d_source,
tl_d_i.d_sink  ,

                tl_d_i.d_data  ,

                (tl_d_i.d_opcode == tlul_pkg::AccessAckData) ? tl_d_i.d_data : '0 ,

Hey guys, sorry I am a bit late to the party here. This is related to the
patch in #1230 https://github.com/lowRISC/opentitan/pull/1230.
In our style guide update, we say
To comply with this style, RTL must place ASSERT_KNOWN assertions on all module outputs, with the exception of signals that may implicitly be X` at the
beginning of the simulation, such as FIFO, SRAM or register file outputs.
This leaves room to omit assertions for cases where the output is X due
to the storage element itself. However, the issue here is that the X are
present within parts of valid transactions stored in the FIFO. We could
make an exception for TL since this is a special case, but in general I
think we should keep these assertions within the primitive itself. How
about introducing a parameter that allows to enable/disable this assertion
upon instantiation (using a conditional generate block)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lowRISC/opentitan/pull/1236?email_source=notifications&email_token=AAH2RSW7ZAG6KFUOVKVU2ODQ3ZGAZA5CNFSM4J5UQ5MKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQRSEZI#discussion_r362620973,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAH2RSX5UBOJLB3TSGFESYTQ3ZGAZANCNFSM4J5UQ5MA
.


Show goal on tier card

Require #1978
On the new tiers page https://opencollective.com/:collective/tiers that list all collective's tiers, we want to display the goal progress for all tiers.
The design for the progress bar should follow the one used for new tier cards so that we can re-use it between components (see https://www.figma.com/file/e71tBo0Sr8J7R5n6iMkqI42d/OC.COM-07-%2F-Collectives?node-id=2151%3A0).
That is to say, we should have the same TierGoalProgress component to re-use the logic.

Resolved in the new collective page


Grammatical changes to crafting enums; Small grammatical change to config.

It seems that something is wrong with your PR. Did you make your pr from the development branch? I can see many changes in your PR that are already in development


Set instance_uid by Server on conflict or request for generation

This is a first approach towards #56
It assumes that Agent's instance_uid can always be updated by Server (since resolving potential issues with conflicts is important).
There's a gap with multiplexed websocket connections which perhaps could be handled by adding Agent capability flag indicating if overrides can be accepted or not

There's a gap with multiplexed websocket connections

Yeah, this is unfortunate. To avoid this problem perhaps we can do slightly differently. Instead of using empty instance_uid field in AgentToServer message as an indication of a request to assign a new instance_uid we can have an explicit flag in AgentToServer to indicate it. In that case the instance_uid field in AgentToServer message may still be set to some temporary value by the Agent until it gets the new instance_uid value from the Server.
Another approach is for the terminating proxy to NOT multiplex connections with empty incoming instance_uid field with other connections. So the initial assignment of instance_uid will require a dedicated outgoing connection from the proxy to the Server. After the assignment of the instance_uid the dedicated connection can be dropped and the multiplexing can start.

which perhaps could be handled by adding Agent capability flag indicating if overrides can be accepted or not

I am not sure how this will solve the problem. Can you expand?

There's a gap with multiplexed websocket connections

Yeah, this is unfortunate. To avoid this problem perhaps we can do slightly differently. Instead of using empty instance_uid field in AgentToServer message as an indication of a request to assign a new instance_uid we can have an explicit flag in AgentToServer to indicate it. In that case the instance_uid field in AgentToServer message may still be set to some temporary value by the Agent until it gets the new instance_uid value from the Server.

I thought about it and I am leaning towards that approach. Will update the PR

which perhaps could be handled by adding Agent capability flag indicating if overrides can be accepted or not

I am not sure how this will solve the problem. Can you expand?

This would merely inform that it is not possible to override the agent instance_id. I think the idea proposed above (temporary id's and explicit flag) is a better one

@pmm-sumo will you be able to submit a corresponding change in opamp-go client and server implementations?

@tigrannajaryan sure thing, will have it later today


Improved side bar, posts, and others

Hi.
I improved the posts, added a couple of widgets to the sidebar, and added a notice for the "wrapper background" form.

Going to be closing this, please separate out each change as seperate PRs so it is easier to discuss each individually.
Some comments however on some of your changes:
Please don't change the existing file indentation or the double/single quotes. I know that it is inconsistent, but I'd rather just keep the existing style until I can figure out what writing style should be used.
The addition of text explaining how to set image backgrounds would make sense if the current system is going to be kept, but there are numerous inherent problems with how it works (namely, that it is a generic text input instead of some form of selector for type that activates a more specific input field) so I'll eventually be rewriting that bit so such a warning isn't necessary anyway.


non-UB bug

The behavior of the test case below is changed by Souper even with UB turned off. Of course it could be an LLVM bug exposed by Souper, but we need to take a look.
$ cat small.c
int printf(const char *, ...);
int x0, x1, x2, x3, x4, x5;
int main() {
  if (x1 <= 0 == 0) {
    x2 = x5 = x0 >= 2 ?: 0 < x0;
    if (x1 >= x5)
      x3 = 0;
  }
  x4 = x2;
  x1 || printf("x\n");
  return 0;
}
$ /home/regehr/f/souper-regehr/third_party/llvm/Release/bin/clang -O3 small.c -o small2 -Xclang -load -Xclang /home/regehr/f/souper-regehr/build/libsouperPass.so -mllvm -z3-path=/usr/bin/z3 -mllvm -solver-timeout=15 -mllvm -souper-debug -mllvm -souper-external-cache -mllvm -souper-exploit-ub=false

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache

; Replacing "  %12 = icmp slt i32 %1, %11"
; from ""
; with "i1 false" in:
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 0:i1
%3:i32 = var
%4:i1 = slt 1:i32, %3
blockpc %0 0 %4 0:i1
blockpc %0 1 %2 0:i1
%5:i1 = slt 0:i32, %3
%6:i32 = zext %5
%7:i32 = zext %4
%8:i32 = phi %0, %6, %7
%9:i1 = slt %1, %8
cand %9 0:i1

; Replacing "  %15 = icmp eq i32 %1, 0"
; from ""
; with "i1 false" in:
%0 = block 3
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 1:i1
%3 = block 2
blockpc %3 0 %2 0:i1
blockpc %3 1 %2 0:i1
%4:i32 = var
%5:i1 = slt 0:i32, %4
%6:i32 = zext %5
%7:i1 = slt 1:i32, %4
%8:i32 = zext %7
%9:i32 = phi %3, %6, %8
%10:i1 = slt %1, %9
blockpc %0 2 %10 0:i1
%11:i1 = eq 0:i32, %1
cand %11 0:i1

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache
$ ./small2
$ clang small.c -o small2
$ ./small2
x

I tried looking at this, but I have a very hard time relating
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 0:i1
%3:i32 = var
%4:i1 = slt 1:i32, %3
blockpc %0 0 %4 0:i1
blockpc %0 1 %2 0:i1
%5:i1 = slt 0:i32, %3
%6:i32 = zext %5
%7:i32 = zext %4
%8:i32 = phi %0, %6, %7
%9:i1 = slt %1, %8

to the only fragment of IR it seems to map to:
entry:
  %0 = load i32* @x1, align 4, !tbaa !2
  %cmp = icmp slt i32 %0, 1
  br i1 %cmp, label %entry.if.end10_crit_edge, label %if.then

if.then:                                          ; preds = %entry
  %1 = load i32* @x0, align 4, !tbaa !2
  %cmp3 = icmp sgt i32 %1, 1
  %conv4 = zext i1 %cmp3 to i32
  br i1 %cmp3, label %cond.end, label %cond.false

cond.false:                                       ; preds = %if.then
  %cmp5 = icmp sgt i32 %1, 0
  %conv6 = zext i1 %cmp5 to i32
  br label %cond.end

cond.end:                                         ; preds = %cond.false, %if.then
  %cond = phi i32 [ %conv6, %cond.false ], [ %conv4, %if.then ]
  store i32 %cond, i32* @x5, align 4, !tbaa !2
  store i32 %cond, i32* @x2, align 4, !tbaa !2
  %cmp7 = icmp slt i32 %0, %cond

The phi node corresponds to the branch in the if.then block, which check the value of %cmp3. This can only correspond to %4:i1 = slt 1:i32, %3 in the souper code. Yet, only one of the blockpc's that follows %4 refers to this value. I would actually expect something like:
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
pc %2 1:i1
%3:i32 = var
%4:i1 = slt 1:i32, %3
blockpc %0 0 %4 0:i1
blockpc %0 1 %4 1:i1
%5:i1 = slt 0:i32, %3
%6:i32 = zext %5
%7:i32 = zext %4
%8:i32 = phi %0, %6, %7
%9:i1 = slt %1, %8

But maybe I'm wrong and simply don't sufficiently understand blockpc's.

Here's the critical transformation. I also have a hard time with blockpc, but it's a really valuable thing.
*** IR Dump After Combine redundant instructions ***
; Function Attrs: nounwind uwtable
define i32 @main() #0 {
  %1 = load i32* @x1, align 4, !tbaa !1
  %2 = icmp slt i32 %1, 1
  br i1 %2, label %._crit_edge, label %3

._crit_edge:                                      ; preds = %0
  %.pre = load i32* @x2, align 4, !tbaa !1
  br label %14

; <label>:3                                       ; preds = %0
  %4 = load i32* @x0, align 4, !tbaa !1
  %5 = icmp sgt i32 %4, 1
  %6 = zext i1 %5 to i32
  br i1 %5, label %10, label %7

; <label>:7                                       ; preds = %3
  %8 = icmp sgt i32 %4, 0
  %9 = zext i1 %8 to i32
  br label %10

; <label>:10                                      ; preds = %3, %7
  %11 = phi i32 [ %9, %7 ], [ %6, %3 ]
  store i32 %11, i32* @x5, align 4, !tbaa !1
  store i32 %11, i32* @x2, align 4, !tbaa !1
  %12 = icmp slt i32 %1, %11
  br i1 %12, label %14, label %13

; <label>:13                                      ; preds = %10
  store i32 0, i32* @x3, align 4, !tbaa !1
  br label %14

; <label>:14                                      ; preds = %._crit_edge, %10, %13
  %15 = phi i32 [ %11, %10 ], [ %.pre, %._crit_edge ], [ %11, %13 ]
  store i32 %15, i32* @x4, align 4, !tbaa !1
  %16 = icmp eq i32 %1, 0
  br i1 %16, label %17, label %18

; <label>:17                                      ; preds = %14
  %puts = tail call i32 @puts(i8* getelementptr inbounds ([2 x i8]* @str, i64 0, i64 0))
  br label %18

; <label>:18                                      ; preds = %14, %17
  ret i32 0
}

; Listing applied replacements for main
; Using solver: Z3 + external cache + internal cache

; Replacing "  %12 = icmp slt i32 %1, %11"
; from ""
; with "i1 false" in:
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 0:i1
%3:i32 = var
%4:i1 = slt 1:i32, %3
blockpc %0 0 %4 0:i1
blockpc %0 1 %2 0:i1
%5:i1 = slt 0:i32, %3
%6:i32 = zext %5
%7:i32 = zext %4
%8:i32 = phi %0, %6, %7
%9:i1 = slt %1, %8
cand %9 0:i1

; Replacing "  %15 = icmp eq i32 %1, 0"
; from ""
; with "i1 false" in:
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 0:i1
blockpc %0 1 %2 0:i1
%3 = block 3
blockpc %3 1 %2 1:i1
%4:i32 = var
%5:i1 = slt 0:i32, %4
%6:i32 = zext %5
%7:i1 = slt 1:i32, %4
%8:i32 = zext %7
%9:i32 = phi %0, %6, %8
%10:i1 = slt %1, %9
blockpc %3 2 %10 0:i1
%11:i1 = eq 0:i32, %1
cand %11 0:i1
*** IR Dump After Souper super-optimizer pass ***
; Function Attrs: nounwind uwtable
define i32 @main() #0 {
  %1 = load i32* @x1, align 4, !tbaa !1
  %2 = icmp slt i32 %1, 1
  br i1 %2, label %._crit_edge, label %3

._crit_edge:                                      ; preds = %0
  %.pre = load i32* @x2, align 4, !tbaa !1
  br label %13

; <label>:3                                       ; preds = %0
  %4 = load i32* @x0, align 4, !tbaa !1
  %5 = icmp sgt i32 %4, 1
  %6 = zext i1 %5 to i32
  br i1 %5, label %10, label %7

; <label>:7                                       ; preds = %3
  %8 = icmp sgt i32 %4, 0
  %9 = zext i1 %8 to i32
  br label %10

; <label>:10                                      ; preds = %3, %7
  %11 = phi i32 [ %9, %7 ], [ %6, %3 ]
  store i32 %11, i32* @x5, align 4, !tbaa !1
  store i32 %11, i32* @x2, align 4, !tbaa !1
  br i1 false, label %13, label %12

; <label>:12                                      ; preds = %10
  store i32 0, i32* @x3, align 4, !tbaa !1
  br label %13

; <label>:13                                      ; preds = %._crit_edge, %10, %12
  %14 = phi i32 [ %11, %10 ], [ %.pre, %._crit_edge ], [ %11, %12 ]
  store i32 %14, i32* @x4, align 4, !tbaa !1
  br i1 false, label %15, label %16

; <label>:15                                      ; preds = %13
  %puts = tail call i32 @puts(i8* getelementptr inbounds ([2 x i8]* @str, i64 0, i64 0))
  br label %16

; <label>:16                                      ; preds = %13, %15
  ret i32 0
}

blockpc asserts path conditions that must be true for a given branch. For example, given the following llvm bitcode:
entry:
    %0 = load i32* @x1, align 4, !tbaa !2
    %cmp = icmp slt i32 %0, 1
    br i1 %cmp, label %entry.if.end10_crit_edge, label %if.then

if.then:                                          ; preds = %entry
    %1 = load i32* @x0, align 4, !tbaa !2
    %cmp3 = icmp sgt i32 %1, 1
    %conv4 = zext i1 %cmp3 to i32
    br i1 %cmp3, label %cond.end, label %cond.false

cond.false:                                       ; preds = %if.then
    %cmp5 = icmp sgt i32 %1, 0
    %conv6 = zext i1 %cmp5 to i32
    br label %cond.end

cond.end:                                         ; preds = %cond.false, %if.then
    %cond = phi i32 [ %conv6, %cond.false ], [ %conv4, %if.then ]
    store i32 %cond, i32* @x5, align 4, !tbaa !2
    store i32 %cond, i32* @x2, align 4, !tbaa !2
    %cmp7 = icmp slt i32 %0, %cond

We start collecting blockpc(s) at the phi instruction %cond. We first look at the first incoming block, i.e., %cond.false. Because %cond.false has single predecessor %if.then and this predecessor's terminator is a conditional instruction, we add one blockpc as blockpc 0 %0 %cmp3 0. Then we examine the predecessor of %if.then. Similarly, because %if.then has single predecessor %entry, which has a conditional terminator, we add another blockpc blockpc %0 0 %cmp 0.
Now we finish the path starting from %cond.false, and we look at %cond's second incoming block %if.then. Similarly, we add blockpc %0 1 %cmp 0.

Does the optimization look correct to you Yang? If so, then LLVM must be wrong somewhere else. I'm not sure how useful bugpoint is for wrong code bugs, anyone know? Otherwise we can do something less smart like cut-and-pasting stuff from the print-after-all output into lli.

@jeroenk For the souper IR that you gave, we cannot have pc, because we don't utilize context-sensitive control-flow information.  In other words, we don't know the fact that %cond.false and %if.then come from the same predecessor %entry with the same branch condition. IMHO, this is probably one of the limitations of our current pc/blockpc harvesting strategy.

@regehr, I will investigate it tonight.

Thanks Yang. Full optimization log is here if that's helpful.
http://pastebin.com/kHLxjv2A

The second optimization looks invalid to me.
%0 = block 2
%1:i32 = var
%2:i1 = slt %1, 1:i32
blockpc %0 0 %2 0:i1
blockpc %0 1 %2 0:i1
%3 = block 3
blockpc %3 1 %2 1:i1
%4:i32 = var
%5:i1 = slt 0:i32, %4
%6:i32 = zext %5
%7:i1 = slt 1:i32, %4
%8:i32 = zext %7
%9:i32 = phi %0, %6, %8
%10:i1 = slt %1, %9
blockpc %3 2 %10 0:i1
%11:i1 = eq 0:i32, %1
cand %11 0:i1

Here we have %10 == 0, which means that %1 < %9 is false, i.e., %1 >= %9. Because %9 can be either 0 or 1, %1 may equal to 0. So, there is some bug in generating blockpc-related queries. Investigating.

Generated KLEE expression. Pondering if it's correct.
  (Or (Eq false
          (And (Or (Eq false (And N0:(Read 0 blockpred) N0))
                   N1:(Eq false (Slt N2:(Read w32 0 a1) 1)))
               (Or N0 N1)))
      (Eq false (Eq 0 N2)))

It's correct, but not sufficient. The problem is that the N0 expression is not being guarded by anything. Hence, if we let
(Slt (Read w32 0 a1) 1)
evaluate to true, which happens when you follow the control flow from entry -> ._crit_edge -> 14 (referring to John's dump above), then you essentially end up with
(And (Eq false N0) N0)
for the (And ...) sub-expression, which can never be satisfied. This is perfectly fine, as we know that the branch on which N0 is needed is not the one being taken. However, this latter fact is not being encoded in the formula.
What exposes the problem here is that in addition to this we have that if
(Slt (Read w32 0 a1) 1)
evaluates to false, then we have a rightful simplification (which is essentially the first simplification souper spots).

I just want to add that if there is a Souper bug here, it's a really hard one to find using Csmith, coming up only once every 12 hours or so on a fast quad core. Next time we might not be so lucky, so let's be really careful about this code.

@jeroenk , Thanks for the explanation! I don't quite understand this part: how can we get
(And (Eq false N0) N0)

by assuming
(Slt (Read w32 0 a1) 1)

to be true. Can you elaborate on that? Thanks.

OK, I see the problem now. Actually the bug relates to the part where we generate souper IR. The phi node relevant to %3 = block is not generated. If we look at the second IR in John's example, we only have %3 = block and blockpc %3, but the corresponding phi instruction doesn't exist. Looks like the way that we compute the equivalence classes for blockpcs has some problem. Working on a fix.

@chenyang78 I assume you figured it out by now, but just for completeness, we have:
(And (Or (Eq false (And N0:(Read 0 blockpred) N0))
                   N1:(Eq false (Slt N2:(Read w32 0 a1) 1)))
               (Or N0 N1)))

By (And x x) = x this is equal to:
(And (Or (Eq false N0:(Read 0 blockpred) )
                   N1:(Eq false (Slt N2:(Read w32 0 a1) 1)))
               (Or N0 N1)))

then assuming (Slt (Read w32 0 a1) 1) we get:
(And (Or (Eq false N0:(Read 0 blockpred) )
                   N1:(Eq false true))
               (Or N0 N1)))

and by (Eq false true) = false:
(And (Or (Eq false N0:(Read 0 blockpred) )
                   N1:false)
               (Or N0 N1)))

Substituting N1:
(And (Or (Eq false N0:(Read 0 blockpred) )
                   false)
               (Or N0 false)))

and simplifying (Or x false) to x we get:
(And (Eq false N0:(Read 0 blockpred) )
               N0))

@jeroenk Thanks!

First of 5 problems found by Csmith ni a 24-hour run.
regehr@regehr-M51AC:reduce_wrong_823724$ SOUPER_NO_SOUPER=1 /home/regehr/f/souper-yang/build/sclang -O2 small.c -o small1 ; ./small1
y
regehr@regehr-M51AC:reduce_wrong_823724$ gcc -O2 small.c -o small1 ; ./small1y
regehr@regehr-M51AC:reduce_wrong_823724$ /home/regehr/f/souper-yang/build/sclang -O2 small.c -o small1 ; ./small1x
regehr@regehr-M51AC:reduce_wrong_823724$ cat small.c
int printf(const char *, ...);
int x0, x1, x2, x3;
int main() {
  if (x3) {
    x1 = x3;
    for (; x0 <= 0; x0++)
      ;
  }
  x2 = x1 |= x0;
  if (x2)
    printf("x\n");
  else
    printf("y\n");
  return 0;
}

$ cat small.c
int printf(const char *, ...);
char x0;
int x1 = 1;
int x2() { return x0 ?: 1 % x0; }

int main() {
  if ((x1 || x2()) ^ 0) {
    if (x1 < 1) {
      ;
    } else {
      printf("x\n");
    }
  }
  return 0;
}
$ /home/regehr/f/souper-yang/build/sclang -O2 small.c -o small1 ; ./small1
$ /home/regehr/f/souper-yang/build/sclang -O0 small.c -o small1 ; ./small1
x
$ gcc -O2 small.c -o small1 ; ./small1
x

Program from the previous comment is a good one since it results in only one souper optimization.
%0 = block 2
%1:i32 = var
%2:i1 = eq 0:i32, %1
blockpc %0 0 %2 1:i1
%3:i8 = var
%4:i1 = eq 0:i8, %3
blockpc %0 0 %4 1:i1
blockpc %0 1 %2 1:i1
%5 = block 2
%6:i32 = sext %3
%7:i32 = srem 1:i32, %6
%8:i32 = phi %0, %7, %6
%9:i1 = eq 0:i32, %8
%10:i1 = phi %5, 0:i1, %9
%11:i1 = slt %1, 1:i32
%12:i1 = or %10, %11
cand %12 1:i1

int printf(const char *, ...);
int x0, x1, x2, x3;
volatile int x4;
void x5(x6) {
  x1 = 0;
  for (; x1 >= 0; x1--)
    if (x4) {
      x6 = x0;
      if (x6)
        x2 = 0;
    }
  x3 = x6 >= 1;
}

int main() {
  x5(0);
  printf("%x", x3);
  return 0;
}

int printf(const char *, ...);
int x0, x1, x2, x3, x4, x5;
char x6;
int main() {
  x6 = x4;
  if (x6) {
    x5 = x1 == 0 ?: x4 % x1;
    if (x5)
      x4 = 0;
  } else {
    if (x0)
      x4 = 9;
    x3--;
  }
  x2 = 1 <= x4;
  printf("%x", x2);
  return 0;
}

int printf(const char *, ...);
int x0, x1, x2, x3, x4, x5;
short x6;
void x7() {
  x2 = 5;
  for (; x2; x2--) {
    x6 = x4 ?: x1 > x4;
    x5 = x6 == 0 ?: x0 / x6;
    if (x5)
      break;
  }
}

int main() {
  x7();
  if (x4)
    for (; x3;)
      printf("x\n");
  return 0;
}

I will investigate those. Thanks, John.

OK, I see Souper's issue for your second program, i.e,
int printf(const char *, ...);
char x0;
int x1 = 1;
int x2() { return x0 ?: 1 % x0; }

int main() {
  if ((x1 || x2()) ^ 0) {
    if (x1 < 1) {
      ;
    } else {
      printf("x\n");
    }
  }
  return 0;
}

Here Souper wrongfully exploited the "UB" in the function x2. The program is UB-free, but Souper is not aware of the fact that x2() will never get invoked. The following simplified souper IR illustrates the problem:
%0 = block 2
%3:i8 = var
%4:i1 = eq 0:i8, %3
blockpc %0 0 %4 1:i1
%6:i32 = sext %3
%7:i32 = srem 1:i32, %6
%8:i32 = phi %0, %7, %6
...

blockpc %0 0 %4 1:i1 asserts that %3 is 0, and therefore %6 is 0. Consequently, we get UB for instruction %7.

I believe that all of these problems occur when -souper-exploit-ub=true but please double check.

Ok I double checked and this optimization does happen when UB is turned off:
$ ~/f/souper-yang/build/souper -souper-exploit-ub=false -stp-path=/usr/local/bin/stp small.bc
; Listing valid replacements.
; Using solver: STP + internal cache

; Static profile 1
; Function: main
%0 = block 2
%1:i32 = var
%2:i1 = eq 0:i32, %1
blockpc %0 0 %2 1:i1
%3:i8 = var
%4:i1 = eq 0:i8, %3
blockpc %0 0 %4 1:i1
blockpc %0 1 %2 1:i1
%5 = block 2
%6:i32 = sext %3
%7:i32 = srem 1:i32, %6
%8:i32 = phi %0, %7, %6
%9:i1 = eq 0:i32, %8
%10:i1 = phi %5, 0:i1, %9
%11:i1 = slt %1, 1:i32
%12:i1 = or %10, %11
cand %12 1:i1

Here's the llvm:
; ModuleID = 'small.c'
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@x1 = global i32 1, align 4
@x0 = common global i8 0, align 1
@str = private unnamed_addr constant [2 x i8] c"x\00"

; Function Attrs: nounwind readonly uwtable
define i32 @x2() #0 {
  %1 = load i8* @x0, align 1, !tbaa !1
  %2 = sext i8 %1 to i32
  %3 = icmp eq i8 %1, 0
  br i1 %3, label %4, label %6

; <label>:4                                       ; preds = %0
  %5 = srem i32 1, %2
  br label %6

; <label>:6                                       ; preds = %0, %4
  %7 = phi i32 [ %5, %4 ], [ %2, %0 ]
  ret i32 %7
}

; Function Attrs: nounwind uwtable
define i32 @main() #1 {
  %1 = load i32* @x1, align 4, !tbaa !4
  %2 = icmp eq i32 %1, 0
  br i1 %2, label %3, label %10

; <label>:3                                       ; preds = %0
  %4 = load i8* @x0, align 1, !tbaa !1
  %5 = sext i8 %4 to i32
  %6 = icmp eq i8 %4, 0
  br i1 %6, label %7, label %x2.exit

; <label>:7                                       ; preds = %3
  %8 = srem i32 1, %5
  br label %x2.exit

x2.exit:                                          ; preds = %3, %7
  %9 = phi i32 [ %8, %7 ], [ %5, %3 ]
  %phitmp = icmp eq i32 %9, 0
  br label %10

; <label>:10                                      ; preds = %0, %x2.exit
  %11 = phi i1 [ false, %0 ], [ %phitmp, %x2.exit ]
  %12 = icmp slt i32 %1, 1
  %or.cond = or i1 %11, %12
  br i1 %or.cond, label %14, label %13

; <label>:13                                      ; preds = %10
  %puts = tail call i32 @puts(i8* getelementptr inbounds ([2 x i8]* @str, i64 0, i64 0))
  br label %14

; <label>:14                                      ; preds = %10, %13
  ret i32 0
}

; Function Attrs: nounwind
declare i32 @puts(i8* nocapture readonly) #2

attributes #0 = { nounwind readonly uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #1 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #2 = { nounwind }

!llvm.ident = !{!0}

!0 = metadata !{metadata !"clang version 3.6.0 (trunk 223586)"}
!1 = metadata !{metadata !2, metadata !2, i64 0}
!2 = metadata !{metadata !"omnipotent char", metadata !3, i64 0}
!3 = metadata !{metadata !"Simple C/C++ TBAA"}
!4 = metadata !{metadata !5, metadata !5, i64 0}
!5 = metadata !{metadata !"int", metadata !2, i64 0}

Below is a partial annotation of the Souper. It looks to me like the blockpcs are telling us that x1 == 0 regardless of which branch was taken. Is that correct, Yang?
%0 = block 2
%5 = block 2

%1:i32 = var               ; x1
%2:i1 = eq 0:i32, %1       ; x1 == 0
%3:i8 = var                ; x0
%4:i1 = eq 0:i8, %3        ; x0 == 0
%6:i32 = sext %3           ; x0 promoted to int
%7:i32 = srem 1:i32, %6    ; 1 % x0

blockpc %0 0 %2 1:i1       ; x1 == 0 if pred 0
blockpc %0 1 %2 1:i1       ; x1 == 0 if pred 1
blockpc %0 0 %4 1:i1       ; x0 == 0 if pred 0
%8:i32 = phi %0, %7, %6    ;

%9:i1 = eq 0:i32, %8       ;
%10:i1 = phi %5, 0:i1, %9
%11:i1 = slt %1, 1:i32     ; x1 < 1
%12:i1 = or %10, %11       ;
cand %12 1:i1

Going by Yang's earlier explanation, I think this says that for %8:i32 = phi %0, %7, %6, we may assume that:

For the the first alternative of phi node: %4 == 1 and %2 == 1
For the second alternative of phi node: %2 == 1

Jeroen that is my understanding as well. The problem here, of course, is that x1 != 0.

But, %8:i32 = phi %0, %7, %6 corresponds to %9 = phi i32 [ %8, %7 ], [ %5, %3 ] in the bitcode, which is only reachable in case x1 == 0. Or am I missing something?

Jeroen, I think you're right. So I'm still not sure what's going wrong here.
One thing we should consider is what the no-UB mode does when there is a divide by zero. My understanding is that division by zero is permitted in SMTLIB2 code but that the result is unspecified. Would that be contributing to the problem here?

Thanks, John and Jeroen. I will take a close look at it tonight.

I'm pretty sure the divide by zero isn't the problem...

Div by zero records UB if the second op is a constant 0. In turn, KLEEBuilder returns constant 0 as the result. If any path conditions constrain the second op to be 0, solver doesn't complain. I did the following test:
%0:i32 = var
pc %0 0:i32
%1:i32 = udiv 1:i32, %0
cand %1 1:i32

$ souper-check $SOUPER_SOLVER -souper-exploit-ub=1 divbyzero.opt
; LGTM

$ souper-check $SOUPER_SOLVER -souper-exploit-ub=0 divbyzero.opt
Invalid, e.g.

%0 = 0

@regehr Bitvector division in SMTLib is a total function, so anything divided by zero will have a well-defined answer. However, the solver is not allowed to make any assumptions about the result of the function in case the divisor is 0.
For a program in which the division by zero cannot actually be executed, I would expect the SMT query that is generated to be such that the value of the division does not matter for the satisfiability of the query.

Thanks @jeroenk. I ran the test above with z3. stp says in both cases LGTM.

The KLEE expression is as follows (writing A ==> B for (Or (Eq false A) B)) and with some trivial simplification:
(And (N0:(Read 0 blockpred0) ==> (And N1:(Eq 0 N2:(Read w32 0 a1)) (Eq 0 N3:(Read w8 0 a3))))
     (Or N0 N1))

    ==>

(Or (And (Eq false (Read 0 blockpred)) (Eq 0 (Select w32 N0 (SRem w32 1 N4:(SExt w32 N3)) N4)))
    (Slt N2 1))

Here, N0 represents the blockpc, N2 represents %1 (or x1 in the original C code), and N3 represents %3 (or x0 in the original). For the optimisation to hold the expression must be valid, i.e., whatever values I pick for N0, N1, and N2, the result must be true. Of course, this is not what we want in this case.
Let's concentrate on the case that N2 != 0 (I believe that the N2 == 0 case is properly encoded by the expression). If N2 != 0, the N1 == false, and the expression simplifies to:
(And (N0:(Read 0 blockpred0) ==> false)
        N0)

    ==>

(Or (And (Eq false (Read 0 blockpred)) (Eq 0 (Select w32 N0 (SRem w32 1 N4:(SExt w32 N3)) N4)))
    (Slt N2 1))

The second half of the formula simplifies depending on N2 >= 1, or N2 < 0, but this doesn't really matter, as
(And (N0:(Read 0 blockpred0) ==> false) N0)

is always false. And, hence, the whole formula is true if N2 != 0. So, N0 affects the validity of the expression even in case N2 != 0, and this should not happen, as it's related to the execution path we didn't choose in this case! It seems that there's something missing from the generated KLEE expression, but I'm not totally sure what that is (needs more thought).

I think I'm missing here something, because for me the souper optimization output looks correct. Regardless the branch of %8:i32 = phi %0, %7, %6, %1 is always constrained to 0 (blockpcs are equal for both branches). Consequently, %11 is always 1 and herewith cand %12 1:i1 is correct. In other words, the optimization is equivalent to the opt with all blockpcs replaced with a single pc %2 1:i1. What is wrong with my reasoning?

Raimondas, if you can't find a bug in the Souper optimization, there must be a harvesting bug, right?

@rsas Naive question: wouldn't constraining %1 to 0 require blockpc's that refer to %5?

Yes, if others prove me wrong. I don't see a bug in the Souper output and I see a problem in the LLVM as follows:
The first branching instruction br i1 %2, label %3, label %10 can jump to label %10 if %1 != 0. I hope we all agree here.
At label 10, the execution of %or.cond = or i1 %11, %12 is constrained by blockpcs (collected through %11) which in turn constrain %1. This is not ok, because there are no constraints on %1 when coming from the initial false branch and jumping to label 10.

In summary, %12 can be 0 or 1 when coming from the initial false branch. On that path, there are no constraints on %1!

Thanks guys. @rsas, I think you are right. Looks like it relates to issue #157. We shouldn't skip constraints on branches, which can result in both missing-optimization and mis-optimization.

@jeroenk can you please rephrase the question?

@rsas Rephrasing: I think I'm fundamentally confused about what a list of souper instructions is. From the documentation and all previous discussions I got the impression that there's some importance in the order of the instructions, but following your reasoning above, it seems that I should view as a set and not as a list. We write it as a list to conveniently express some non-circular dependencies that may occur.

Correct. You always start from the bottom cand instruction, collect all constraints recursively and add them at the top level.

OK, I think the fundamental problem here is that we are not taking control-flow facts into account. For the given example, two blockpcs on %2 cannot be viewed as a pure "union".  Thinking about a way to encode control-flow info...

I think so too. I suspect you'll need to encode both data flow and control
flow information into blockpc.
On Mon, Feb 9, 2015 at 6:27 PM, Yang Chen<EMAIL_ADDRESS>wrote:

OK, I think the fundamental problem here is that we are not taking
control-flow facts into account. For the given example, two blockpcs on %2
cannot be viewed as a pure "union". Thinking about a way to encode
control-flow info...
—
Reply to this email directly or view it on GitHub
https://github.com/google/souper/issues/166#issuecomment-73633209.

@chenyang78 I think it's time for us to rethink the design of pcs/blockpcs/UBs/Phis. They all have things in common and the code is not trivial at all.

At the same time, we have to encode poison values too.

@rsas I agree.

One of the earliest major Souper bugs was where UB constraints weren't properly filtered through phi nodes. Raimondas, you solved that -- can you please explain the flavor of the solution and which aspects of it can be used (and not used) here?
Regarding poison/undef/UB, this is hard, but Nuno implemented a good solution in Alive. I'll mail everyone our draft Alive paper where this is explained pretty carefully. We're going to need to do something similar here, but note that there are some differences between poison/undef/UB as they are explained in the LangRef and as they really are treated by LLVM. It sucks to not have a real specification.

The flow of UB constraints is encoded using Phi branch predicates. We add the UBs only if the corresponding Preds are true (Pred => UB). I'm not sure if we can easily adapt this concept to blockpcs though.

Do we want to add a flag for turning off blockpc until we can resolve this?

I second that.

blockpc constraints are generated in the same way as UBs. I will add a flag to disable blockpc for now.

Thanks Yang.
Yes, I feel like there can be some shared mechanisms here. Let's take the time and get this right -- in the meantime Souper works perfectly well in degraded (no UB, no blockpc) mode.

@chenyang78 I just created a pull request that clears the collected BPCs so that they are neither printed nor exploited. If we disable them in KLEEBuilder, they would be still printed which might be confusing. Let me know what you think.

@rsas Thanks for implementing it. I was considering to disable blockpc by default. It's fine to make it on by default. Enabling blockpc by default requires fewer changes to the relevant tests.

Leaving the default option true encourages us to work on the fix faster ;)

I was going to say the same thing.

@regehr @rsas. Agreed ;)


Custom fixture teardown isn't called

Describe the bug
The docs suggest using text.extend will allow setup and teardown within the fixture.
https://github.com/vitest-dev/vitest/blob/main/docs/guide/test-context.md#usage
I may have set this up incorrectly, but the teardown doesn't seem to fire after the test finishes.
Reproduction
Stackblitz reproduction, instructions to see error in README.md
https://stackblitz.com/edit/node-3ln3ly?file=.stackblitzrc,example.test.ts,package.json,todos.json,README.md
System Info
System:
    OS: macOS 13.2.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 107.80 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 20.5.0 - ~/.nvm/versions/node/v20.5.0/bin/node
    npm: 10.1.0 - ~/node_modules/.bin/npm
    pnpm: 8.8.0 - ~/Library/pnpm/pnpm
    Watchman: 2<IP_ADDRESS> - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 117.0.5938.88
    Safari: 16.3
  npmPackages:
    vite: ^4.4.11 => 4.4.11
    vitest: ^0.34.6 => 0.34.6

Used Package Manager
pnpm
Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

This looks like a bug! @Dunqing 👀

After a bit of digging, it looks like it might be related to this change #4168 as this works in the last patch.
I would have expected this test to catch this bug! https://github.com/vitest-dev/vitest/blob/ec2e80406555d1eaceefc261d7a50675c2363882/test/core/test/test-extend.test.ts#L303C21-L303C21

This is a bug and will fixed in #4250. There is a workaround for you can write a test inside describe will work


Performance gap with Llama-2-7B

Sorry to bother you, I run the weight-only quantization evaluation with the command below:
CUDA_VISIBLE_DEVICES=0 python main.py
--model /PATH/TO/LLaMa/Llama-2-7b --eval_ppl --net Llama-2-7b
--epochs 0 --output_dir ./log/Llama-2-7b-w3a16g128test
--wbits 3 --abits 16 --group_size 128 --lwc --resume log/llama-2-7b-w3a16g128/Llama-2-7b-w3a16g128.pth
But I encounter the problem that Attention mask size mismatch error in Line156 in init_llama_layer.py, then I change "attention_mask=attention_mask" in Line247 with "attention_mask=attention_mask[..., :-1]", the performance is:
INFO wikitext2 : 6.209373950958252
INFO c4 : 7.872798919677734
a bit higher than the results in paper, could you help me with this issue? Thank you!

The checkpoints have some mismatch with current code.
Retrain by yourself through current code can successfully reproduce the results.


Done

@swapnilsparsh  I have created pagination without creating extra index files

@Bharadwajshivam28 Check your changes here
Why there is showing index4.html ?

@Bharadwajshivam28 Check your changes here
Why there is showing index4.html ?

There is no index and for one - two there is that also in folder

@Bharadwajshivam28 Check your changes here
Why there is showing index4.html ?

I tried by removing index completely but for the links I need good webpage link so that is costly and all.. pasting the link simple after running doesn't work sometime

@Bharadwajshivam28 Check your changes here
Why there is showing index4.html ?

So I have stored these in folders..like PROJECTS and index file

@Bharadwajshivam28 Check your changes here
Why there is showing index4.html ?

can it be like named as page1,page2 and like because working with links is getting messed . It doesnt have a proper link so sometimes it doesnt work

so i have put these in folders

@Bharadwajshivam28 You dont have to make new folders or new files.
Make changes in index.html only and there is folder 30DaysOfJavaScript where there are 2 files script.js and style.css
Only in these files you have to make changes.

ok sir

sir can i host website by netfily and then insert the link

hositng the remaing pages like2,3,4,5,6 and 7

@Bharadwajshivam28
Check here
When you change the page then the url does not change only the card gets updated.

because without creating files we need a proper link so i can host it using netfily and apply the link

ok sir i will now do it

@swapnilsparsh Sir can i host the remaing pages by netfily

Like i can get the link of page 2,3,4,5,6 and other

You don't have to host the pages.
It will be automatically hosted when you commit changes

sir like i am asking that without creating files

when i give the links then sometime it run and soemtime it doesnt

and when i am creating folders or file then doing then it is done ccorrectly

@swapnilsparsh so please suggest me like how because creating folder or file then you dont want it sir so please sugeest

@swapnilsparsh so please suggest me like how because creating folder or file then you dont want it sir so please sugeest

Told you here
https://github.com/swapnilsparsh/30DaysOfJavaScript/pull/1009#issuecomment-1445915261

sir i understood that but without creating files or folders

like i am not able to understand that without creating seprate files how it will be done

You have to refresh the Div Tag inside which all the cards are there.

Basically i have idea like we can create folders and each folder will have its 30 project and that will be clean and simple stuff

like i am not able to understand that without creating seprate files how it will be done

You have to refresh the Div Tag inside which all the cards are there.

elaborate it sir some more

@swapnilsparsh Sir i not have much depth knowledge of these stuff

SO can i do it by creating seprate folders

I have spent whole night sir yesterday so please let me do it

@Bharadwajshivam28 Appreciate your efforts but this is not the correct way to do so.

ok sir

sir please reach my one blog issue


Support for positional argument

Just a random thought that if we could support positional arguments so that users can do
sos-do something

when sos-do is a sos script with #!/usr/bin/env sos-runner.
Note that currently the command line only supports - options from sos, and -- for parameters, and does not allow positional argument.

We currently have sos run script.sos step it is already 3 positional arguments ... I think it is too much for additional ones.

Yes, technically speaking this is not possible since step is optional so SoS would not be able to differentiate between
sos-do workflow
sos-do param
sos-do workflow param


Improve travis build

Coverage increased (+0.01%) to 70.254% when pulling e3f207a3d0018f769aa6ef96e7bfa5534ff4c30c on improve_travis_build into 1d4a9191af70cb37157a4be7814557f7ea7effe3 on dev.


Describe steps needed to release

Describe the steps needed to release.

What is the expected result of a release?

Released jar in Maven Central
Tagged release in Git repo

Which Gradle commands are needed to get there?
What to do after the release?

README.md updates
announce new release somewhere?

Sounds about right. I guess we could create a Wiki page for others interested in doing new releases. They'd need to be given admin rights and also they'd need to sign up to sonatype and given access.

Extended the file release-procedure.md a bit. Contains enough details to get started.


Postgres Database Migration

Fully transferring from SQLite to PostgreSQL

New Database is hosted at neon.tech

Connection String for test database is provided in the dc server or create your own testing db at neon.tech


Implement Queen's Men

@ystros: As we talked about in chat, showing the hand through a prompt instead of a chat message would help with an eventual melee implementation. Not sure if I like how this turned out though. Clicking a character card has the same effect as clicking Done, which seems weird but I don't know a way around this.  On the other hand, this way does take relatively few clicks.
Another possibility: reveal the hand via chat message. Then, if you have a standing non-Bara character, prompt whether to kneel one, and if so, only prompt non-character cards from the opponents hand to be discarded. Showing the hand through a chat message can't be used in melee obviously, unless we would implement player specific chat messages (and the ability to whisper in melee would be cool aside from this). Aside from that, this is much how we would do other cards, with costs paid before deciding on effects, but takes either 0,1 or 3 clicks to resolve after triggering. The current implementation takes 1 or 2 clicks to resolve after triggering.
Finally, is there a way to pass a card object through a menuPrompt instead of a cardId as I do now?

I think implementation wise this is fine for now. It certainly is awkward, but most of the other solutions involve more clicks or a new prompt type.
I'd like to experiment eventually with a prompt that shows the cards directly (for things like this and search prompts) and allows them to be selected like any other card. That would make a prompt like this a bit less awkward. But I know there's been resistance to this type of prompt. 3-ish minute mockup:

For the menu prompt, there isn't a way to get the object other than how you've done it. We should put together a new 'card menu' prompt at some point that does the card look up for you (and could be then converted to the style prompt above if we ever decide to go in that direction).